Add ccache-remote extension for distributed compilation caching via Redis #9168
Conversation
📝 WalkthroughWalkthroughAdds a new ccache-remote extension that discovers a remote Redis-backed ccache via mDNS (ccache.local) or configured endpoint, exports CCACHE_* vars into Docker and kernel/U‑Boot make environments, enables USE_CCACHE, and reports remote cache stats at build end while falling back to local ccache if unavailable. Changes
Sequence Diagram(s)sequenceDiagram
participant Host
participant MDNS as mDNS/Avahi
participant Redis
participant Docker
participant Make as Kernel/U-Boot Make
Host->>MDNS: resolve "ccache.local"
alt resolved
MDNS-->>Host: IP
Host->>Redis: probe (get_redis_stats via redis-cli || netcat)
alt reachable
Host->>Host: set CCACHE_REMOTE_STORAGE and populate CCACHE_* passthrough
Host->>Docker: add CCACHE_* via DOCKER_EXTRA_ARGS
else unreachable
Host->>Host: leave remote unset (use local ccache)
end
else not resolved
MDNS-->>Host: not found → use local ccache
end
Docker->>Make: launch container with env -i including CCACHE_* (if set)
Make->>Make: call_extension_method "kernel_make_config"/"uboot_make_config"
alt remote set
Make->>Make: include CCACHE_REMOTE_STORAGE in make envs
end
Make->>Host: trigger final stats callback
Host->>Redis: get final remote stats
Host-->>Make: log remote reads/writes/errors and hit rate
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧠 Learnings (16)📓 Common learnings📚 Learning: 2025-09-24T09:54:07.968ZApplied to files:
📚 Learning: 2026-01-03T20:44:12.559ZApplied to files:
📚 Learning: 2025-09-22T06:01:50.284ZApplied to files:
📚 Learning: 2025-12-18T23:40:41.627ZApplied to files:
📚 Learning: 2025-04-28T08:27:26.890ZApplied to files:
📚 Learning: 2025-09-22T21:59:36.334ZApplied to files:
📚 Learning: 2025-12-13T11:39:08.046ZApplied to files:
📚 Learning: 2025-03-31T22:20:41.849ZApplied to files:
📚 Learning: 2025-09-14T06:32:29.806ZApplied to files:
📚 Learning: 2025-11-20T18:20:11.985ZApplied to files:
📚 Learning: 2025-07-25T03:51:50.830ZApplied to files:
📚 Learning: 2025-11-13T14:50:44.662ZApplied to files:
📚 Learning: 2025-09-12T19:28:38.491ZApplied to files:
📚 Learning: 2025-06-22T00:45:26.182ZApplied to files:
📚 Learning: 2025-06-14T05:53:10.627ZApplied to files:
🧬 Code graph analysis (2)lib/functions/host/docker.sh (1)
lib/functions/compilation/uboot.sh (1)
🔇 Additional comments (3)
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/medium
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
extensions/ccache-remote.sh (2)
67-71: Consider adding timeouts toredis-clicommands.The
redis-clicalls lack timeouts, which could cause hangs if the Redis server is slow or unresponsive. While this is only for stats gathering (non-critical), it could delay builds unnecessarily.🔎 Proposed fix to add timeouts
if command -v redis-cli &>/dev/null; then local keys mem - keys=$(redis-cli -h "$ip" -p "$port" DBSIZE 2>/dev/null | grep -oE '[0-9]+') - mem=$(redis-cli -h "$ip" -p "$port" INFO memory 2>/dev/null | grep "used_memory_human" | cut -d: -f2 | tr -d '[:space:]') + keys=$(timeout 2 redis-cli -h "$ip" -p "$port" DBSIZE 2>/dev/null | grep -oE '[0-9]+') + mem=$(timeout 2 redis-cli -h "$ip" -p "$port" INFO memory 2>/dev/null | grep "used_memory_human" | cut -d: -f2 | tr -d '[:space:]')
117-140: Native build fallback logic is sound, but declaration style is inconsistent.The function correctly handles both Docker (where
CCACHE_REMOTE_STORAGEis pre-set via env) and native builds (where mDNS resolution happens here). However, line 120 usesdeclare -gwhile line 133 usesexport—both work but are stylistically inconsistent.🔎 Consistent declaration style
if [[ -n "${ccache_ip}" ]]; then - export CCACHE_REMOTE_STORAGE="redis://${ccache_ip}:6379|connect-timeout=500" + declare -g CCACHE_REMOTE_STORAGE="redis://${ccache_ip}:6379|connect-timeout=500" display_alert "Remote ccache discovered" "${CCACHE_REMOTE_STORAGE}" "info"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
extensions/ccache-remote.shlib/functions/compilation/kernel-make.sh
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: The kernel_config_set_m function in lib/functions/compilation/armbian-kernel.sh preserves existing 'y' (built-in) settings and only sets options to 'm' (module) when they are not already built-in, achieving "prefer modules but allow built-ins" behavior.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-27T21:47:58.020Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-bcm2711-edge.config:859-861
Timestamp: 2025-09-27T21:47:58.020Z
Learning: In the Armbian build system, kernel configuration files in config/kernel/ are generated through an automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-27T21:50:04.845Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sm8250-edge.config:80-82
Timestamp: 2025-09-27T21:50:04.845Z
Learning: In the Armbian build system, kernel configuration files are generated through this automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-27T21:49:55.796Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sm8250-current.config:78-80
Timestamp: 2025-09-27T21:49:55.796Z
Learning: In the Armbian build system, kernel configuration files are generated through an automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
lib/functions/compilation/kernel-make.shextensions/ccache-remote.sh
🧬 Code graph analysis (2)
lib/functions/compilation/kernel-make.sh (1)
lib/functions/general/extensions.sh (1)
call_extension_method(38-60)
extensions/ccache-remote.sh (1)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)
🔇 Additional comments (4)
lib/functions/compilation/kernel-make.sh (1)
73-81: Well-designed extension hook for kernel build customization.The hook placement is appropriate—after all standard environment and parameter setup but before command assembly. The heredoc documentation clearly specifies which arrays are modifiable and which variables are read-only. This follows the established Armbian extension pattern visible in
lib/functions/general/extensions.sh.extensions/ccache-remote.sh (3)
1-60: Comprehensive documentation with appropriate security warnings.The documentation covers usage patterns, server setup, mDNS configuration, and fallback behavior well. The security warning at lines 36-38 appropriately highlights the risks of the example configuration and references official Redis security documentation.
88-115: Host-side mDNS resolution logic is correct.The approach of resolving
ccache.localon the host before Docker launch is appropriate since mDNS typically doesn't work inside containers. The use ofgetent hostsis the right choice for portability across systemd-resolved and libnss-mdns setups. Graceful fallback when the hostname isn't resolvable is handled well.
142-149: Kernel make hook implementation is correct and minimal.The function follows the Armbian extension naming convention (
kernel_make_config__*), correctly modifiescommon_make_envsto inject the remote storage configuration, and uses the same quoting style as other environment additions inkernel-make.sh(e.g., line 34). Based on learnings, functions with this naming pattern are auto-discovered via the extension system.
The difference is intentional:
Both achieve global visibility in the current shell, but export additionally makes the variable available to subprocesses, which is required for CCACHE_REMOTE_STORAGE but unnecessary for USE_CCACHE. |
rpardini
left a comment
rpardini
left a comment
There was a problem hiding this comment.
Very, very nice stuff. Since you're creating a new hook for make params, maybe also a new hook for doing host-side, pre-Docker stuff?
extensions/ccache-remote.sh
Outdated
| # with: avahi-publish-address -R ccache.local <IP>) and passes the resolved IP | ||
| # to Docker container via CCACHE_REMOTE_STORAGE environment variable. | ||
| # mDNS resolution doesn't work inside Docker, so we must resolve on host. | ||
| function add_host_dependencies__setup_remote_ccache_for_docker() { |
There was a problem hiding this comment.
This is the only hook that runs "on the host".
It was meant exclusively for adding host dependencies (so that they can be in the Dockerfile, and thus, cached).
Many people (incl me...) have (ab)used it for other purposes.
Maybe it's time to create a separate one, for cleanliness?
There was a problem hiding this comment.
Personally, many times I wanted to make a hook that allows to change the docker file itself, but I never got the courage.
Many people (incl me...) have (ab)used it for other purposes.
Maybe it's time to create a separate one, for cleanliness?
Where exactly do you think this is appropriate, and with what name?
There was a problem hiding this comment.
I have not been scientific with those names. Usually I find a spot where it makes sense "spot" and then do something like pre_spot_something or post_spot_something.
In this case it could be near where add_host_dependencies is, but I'd leave it up to you. Also could be done in a diff/separate PR, as add_host_dependencies has already other (ab)uses so it's not fair to block this PR on that nit.
There was a problem hiding this comment.
Looking for add_host_dependencies, I see after one assignment host_dependencies_known. If there is no hard-coded policy "these hooks are only for assigning a variable with this name", then this is probably enough.
There was a problem hiding this comment.
yeah, but that's also hostdep-related. And both actually run both in the host and in the container (in different moments). Maybe something like host_pre_docker_launch?
There was a problem hiding this comment.
Maybe something like
host_pre_docker_launch?
I like this idea
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
88-115: Host-side resolution logic is correct.The function correctly resolves ccache.local on the host (where mDNS works) before Docker launch, passes the configuration to Docker via environment variables, and handles fallback gracefully.
Consider validating ccache version and CCACHE_REMOTE_STORAGE format.
The code assumes ccache 4.4+ is available and that CCACHE_REMOTE_STORAGE (if pre-set) is well-formed. While ccache will fail gracefully on misconfiguration, proactive validation could improve the user experience.
Optional: Add validation checks
# After line 90, before the early return: if [[ -n "${CCACHE_REMOTE_STORAGE}" ]]; then # Validate format if [[ ! "${CCACHE_REMOTE_STORAGE}" =~ ^redis:// ]]; then display_alert "Invalid CCACHE_REMOTE_STORAGE format" "${CCACHE_REMOTE_STORAGE}" "wrn" fi # Could also check ccache version here display_alert "Remote ccache pre-configured" "${CCACHE_REMOTE_STORAGE}" "info" declare -g -a DOCKER_EXTRA_ARGS+=("--env" "CCACHE_REMOTE_STORAGE=${CCACHE_REMOTE_STORAGE}") return 0 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.
📒 Files selected for processing (1)
extensions/ccache-remote.sh
🧰 Additional context used
🧠 Learnings (3)
📓 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.
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.sh
🧬 Code graph analysis (1)
extensions/ccache-remote.sh (1)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)
🔇 Additional comments (4)
extensions/ccache-remote.sh (4)
1-60: Excellent documentation.The documentation is comprehensive and clear, covering usage, requirements, security considerations, and setup examples. The security warning about the insecure Redis configuration is appropriate and prominent.
62-81: Stats function is well-designed with good fallback.The function properly handles errors, uses timeouts, and provides a netcat fallback when redis-cli is unavailable. The grep patterns are adequate for well-formed Redis responses.
117-140: Docker/native build configuration is correct.The function correctly:
- Enables ccache unconditionally (line 120)
- Uses
exportfor CCACHE_REMOTE_STORAGE (line 133) to make it available to child processes like ccache- Uses
declare -gfor USE_CCACHE (line 120) since it's an internal Armbian variableThe distinction between
declare -gandexportis intentional and correct, as confirmed by the PR discussion.
142-149: The quoting pattern is correct and requires no changes.The pattern
CCACHE_REMOTE_STORAGE='${CCACHE_REMOTE_STORAGE}'on line 146 matches the established convention used throughoutcommon_make_envs(seeCCACHE_DIR,XZ_OPT,TERM,COLUMNS, etc.). Each array element is a complete assignment statement passed as a separate positional argument torun_host_command_logged, which concatenates them viabash -c "$*". Theenv -icommand then parses these KEY='value' assignments correctly. The single quotes are literal quoting characters intended to protect values containing shell metacharacters (e.g., pipes in Redis URLs). No additional shell expansion occurs that would compromise this pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
65-85: Consider simplifying the stats extraction pipelines.The current implementation uses complex pipelines with
grep,cut, andtrthat could be fragile if Redis output format changes slightly. While functional, consider usingawkfor more robust parsing.🔎 Proposed simplification
function get_redis_stats() { local ip="$1" local port="${2:-6379}" local stats="" if command -v redis-cli &>/dev/null; then local keys mem - keys=$(timeout 2 redis-cli -h "$ip" -p "$port" DBSIZE 2>/dev/null | grep -oE '[0-9]+') - mem=$(timeout 2 redis-cli -h "$ip" -p "$port" INFO memory 2>/dev/null | grep "used_memory_human" | cut -d: -f2 | tr -d '[:space:]') + keys=$(timeout 2 redis-cli -h "$ip" -p "$port" DBSIZE 2>/dev/null | awk '/^[0-9]+$/ {print $1; exit}') + mem=$(timeout 2 redis-cli -h "$ip" -p "$port" INFO memory 2>/dev/null | awk -F: '/^used_memory_human:/ {gsub(/[[:space:]]/, "", $2); print $2}') if [[ -n "$keys" ]]; then stats="keys=${keys:-0}, mem=${mem:-?}" fi else # Fallback: try netcat for basic connectivity check if nc -z -w 2 "$ip" "$port" 2>/dev/null; then stats="reachable (redis-cli not installed for detailed stats)" fi fi echo "$stats" }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
extensions/ccache-remote.shlib/functions/compilation/kernel-make.sh
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.shlib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: The kernel_config_set_m function in lib/functions/compilation/armbian-kernel.sh preserves existing 'y' (built-in) settings and only sets options to 'm' (module) when they are not already built-in, achieving "prefer modules but allow built-ins" behavior.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-27T21:47:58.020Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-bcm2711-edge.config:859-861
Timestamp: 2025-09-27T21:47:58.020Z
Learning: In the Armbian build system, kernel configuration files in config/kernel/ are generated through an automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: In lib/functions/compilation/armbian-kernel.sh, the kernel_config_modifying_hashes array is not universally required for all kernel configuration functions - some functions like armbian_kernel_config__netkit() operate without it, and adding entries with '=m' would be incorrect when the actual result might be '=y' for options already built-in.
Applied to files:
lib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-27T21:50:04.845Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8678
File: config/kernel/linux-sm8250-edge.config:80-82
Timestamp: 2025-09-27T21:50:04.845Z
Learning: In the Armbian build system, kernel configuration files are generated through this automated process: taking previous config → applying scripted changes from armbian-kernel.sh → running kernel's `make oldconfig` → processing by Armbian machinery back into config files. This automated process properly handles kernel configuration dependencies and reduces the likelihood of manual configuration errors.
Applied to files:
lib/functions/compilation/kernel-make.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/compilation/kernel-make.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/compilation/kernel-make.sh
🧬 Code graph analysis (2)
extensions/ccache-remote.sh (1)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)
lib/functions/compilation/kernel-make.sh (1)
lib/functions/general/extensions.sh (1)
call_extension_method(38-60)
🔇 Additional comments (4)
extensions/ccache-remote.sh (3)
87-119: LGTM - Host-based mDNS resolution is appropriate here.The use of the
add_host_dependencieshook for mDNS resolution is justified since mDNS/Avahi resolution doesn't work inside Docker containers. Resolvingccache.localon the host and passing the IP to Docker viaDOCKER_EXTRA_ARGSis the correct approach.The fallback behavior when the Redis server is not found is well-handled with appropriate debug logging.
121-145: LGTM - Intentional use ofdeclare -gvsexport.The distinction between
declare -g USE_CCACHE=yes(line 124) andexport CCACHE_REMOTE_STORAGE=...(line 138) is intentional and correct:
USE_CCACHEis an internal Armbian framework variable that only needs visibility within the shell contextCCACHE_REMOTE_STORAGEmust be inherited by child processes (the ccache binary)The function correctly handles both Docker builds (where the variable is pre-set from the host) and native builds (where discovery happens in the build environment).
147-154: LGTM - Correct integration with the new kernel_make_config hook.This function properly integrates with the new
kernel_make_configextension hook added inlib/functions/compilation/kernel-make.sh. Appending to thecommon_make_envsarray is the intended usage pattern, and it's necessary here because the kernel build usesenv -i, which clears all environment variables.The variable is properly quoted within the string to preserve its value through the environment passing mechanism.
lib/functions/compilation/kernel-make.sh (1)
73-81: LGTM - Well-documented extension hook enables environment customization.The new
kernel_make_confighook is properly positioned and documented. It enables extensions to modify the kernel build environment (viacommon_make_envs) and make parameters (viacommon_make_params_quoted) just before the final make command is assembled.This hook is essential for the ccache-remote extension to inject
CCACHE_REMOTE_STORAGEinto the kernel build environment, working around theenv -ithat clears all environment variables.The heredoc documentation clearly specifies what can be modified and what variables are available read-only, following good extension design practices.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
148-153: Minor edge case in quoting for unusual Redis URLs.The quoting is correct for typical Redis URLs. However, if
CCACHE_REMOTE_STORAGEever contained a single quote character, the inner single quotes would break parsing.🔎 Optional: safer quoting using printf %q
function kernel_make_config__add_ccache_remote_storage() { if [[ -n "${CCACHE_REMOTE_STORAGE}" ]]; then - common_make_envs+=("CCACHE_REMOTE_STORAGE='${CCACHE_REMOTE_STORAGE}'") + common_make_envs+=("CCACHE_REMOTE_STORAGE=$(printf '%q' "${CCACHE_REMOTE_STORAGE}")") display_alert "Kernel make: added remote ccache" "${CCACHE_REMOTE_STORAGE}" "debug" fi }This is a nitpick since standard Redis URLs won't contain single quotes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
extensions/ccache-remote.sh
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.sh
🧬 Code graph analysis (1)
extensions/ccache-remote.sh (1)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)
🔇 Additional comments (4)
extensions/ccache-remote.sh (4)
1-64: Comprehensive documentation with appropriate security warnings.The header documentation is thorough, covering usage examples, configuration format, server/client setup, and importantly includes a security warning about the insecure Redis configuration. This is well done.
66-85: Good defensive implementation with fallback.The function correctly uses timeouts to prevent hangs, handles missing
redis-cligracefully with a netcat fallback, and returns empty on failure allowing callers to handle it appropriately.
92-119: Implementation is sound; hook placement discussed in prior review.The mDNS resolution must happen on the host before Docker starts (since mDNS doesn't work inside containers), making this hook placement practical. The previous discussion about creating a separate hook for non-dependency uses could be addressed in a follow-up PR if desired.
The implementation correctly handles both pre-configured and auto-discovered scenarios with appropriate fallback behavior.
122-144: Correct use ofdeclare -gvsexportfor variable scope.The distinction is appropriate:
USE_CCACHEis an internal framework variable needing only shell visibility, whileCCACHE_REMOTE_STORAGErequiresexportsince the external ccache binary reads it from the environment. This aligns with the explanation in the PR discussion.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
4-5: Consider verifying ccache version at runtime.The documentation specifies ccache 4.4+ is required for remote storage support. While the current graceful fallback handles missing remote storage, an explicit version check could provide clearer feedback to users with older ccache versions.
💡 Optional: Add ccache version check
You could add a version check in
extension_prepare_config__setup_remote_ccache():# Check ccache version if remote storage is configured if [[ -n "${CCACHE_REMOTE_STORAGE}" ]] && command -v ccache &>/dev/null; then local ccache_version ccache_version=$(ccache --version | head -n1 | grep -oE '[0-9]+\.[0-9]+' | head -n1) if [[ -n "${ccache_version}" ]]; then # Compare version (requires ccache 4.4+) if ! version_compare "${ccache_version}" ge "4.4"; then display_alert "ccache version too old for remote storage" "found ${ccache_version}, need 4.4+" "wrn" unset CCACHE_REMOTE_STORAGE fi fi fiHowever, this is purely optional since the fallback behavior already handles incompatible versions gracefully.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
extensions/ccache-remote.shlib/functions/compilation/uboot.sh
🧰 Additional context used
🧠 Learnings (16)
📚 Learning: 2025-11-20T18:20:11.985Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8968
File: patch/u-boot/u-boot-sunxi/arm64-dts-sun50i-h6-orangepi.dtsi-Rollback-r_rsb-to-r_i2c.patch:36-36
Timestamp: 2025-11-20T18:20:11.985Z
Learning: The rewrite-patches tool (REWRITE_PATCHES=yes) in the Armbian build system can inadvertently introduce semantic changes when the u-boot/kernel git base revision differs from expected state. The tool applies patches, commits them, and re-exports them using git format-patch, which can cause the re-exported patch to reflect the base revision's state rather than preserving the original patch intent. This is particularly problematic for device tree changes like interrupt specifications. The tool currently lacks validation mechanisms to detect such semantic drift, and affected patches must be manually corrected after rewriting.
Applied to files:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-12-13T11:39:08.046Z
Learnt from: pyavitz
Repo: armbian/build PR: 9058
File: patch/u-boot/legacy/u-boot-spacemit-k1/003-SpacemiT-K1X-Fixups.patch:28-67
Timestamp: 2025-12-13T11:39:08.046Z
Learning: In the Armbian build system for SpacemiT U-Boot patches (patch/u-boot/legacy/u-boot-spacemit-k1/), alignment with mainline U-Boot behavior is prioritized. For example, in boot mode handling, leaving devnum unchanged in the default case (when devtype is cleared) follows mainline conventions rather than explicitly clearing it to handle edge cases.
Applied to files:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-03-31T22:20:41.849Z
Learnt from: rpardini
Repo: armbian/build PR: 8044
File: patch/u-boot/v2025.04/cmd-fileenv-read-string-from-file-into-env.patch:73-75
Timestamp: 2025-03-31T22:20:41.849Z
Learning: When porting patches between U-Boot versions (like from 2025.01 to 2025.04), rpardini prefers to maintain patches as-is rather than introducing refactoring changes, even when potential improvements are identified. This approach prioritizes consistency and reduces the risk of introducing new issues.
Applied to files:
lib/functions/compilation/uboot.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/compilation/uboot.shextensions/ccache-remote.sh
📚 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:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
lib/functions/compilation/uboot.shextensions/ccache-remote.sh
📚 Learning: 2025-07-25T03:51:50.830Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8428
File: config/boards/lckfb-taishanpi.csc:5-9
Timestamp: 2025-07-25T03:51:50.830Z
Learning: When reviewing PRs in the Armbian build system, U-Boot defconfig files and patches may be added as part of the PR changes but might not be visible in the current repository clone state during review. It's important to check the actual PR file changes directly via GitHub or the PR API to get the complete picture of what files are being added or modified.
Applied to files:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-12-19T13:56:45.124Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 0
File: :0-0
Timestamp: 2025-12-19T13:56:45.124Z
Learning: When reviewing kernel or u-boot version bump PRs in the Armbian build system, check if patches existed in previous kernel version directories (e.g., sunxi-6.12, sunxi-6.13) before describing them as new features. If a patch and the majority of its contents existed previously with no major functionality changes, focus the review on the actual changes: the version bump itself and patch compatibility adjustments. Don't describe existing patches being ported/maintained across versions as new features or drivers—this is misleading. The patches are existing code being re-aligned to work with the new upstream version.
Applied to files:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-11-13T14:50:44.662Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8925
File: lib/functions/host/docker.sh:141-141
Timestamp: 2025-11-13T14:50:44.662Z
Learning: In the Armbian build system, certain boards have U-Boot that doesn't build correctly on ubuntu:noble but works on ubuntu:jammy. For these boards, DOCKER_ARMBIAN_BASE_IMAGE can be set to "ubuntu:jammy" via an extension file loaded before docker_cli_prepare() runs, allowing legacy toolchains to work while the community works on proper fixes with limited resources.
Applied to files:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-09-12T19:28:38.491Z
Learnt from: Grippy98
Repo: armbian/build PR: 8622
File: config/sources/families/k3.conf:66-66
Timestamp: 2025-09-12T19:28:38.491Z
Learning: In the Armbian k3 family build system (config/sources/families/k3.conf), builds do not fail when TIBOOT3_BOOTCONFIG is unset, even though tiboot3.bin is still listed in UBOOT_TARGET_MAP. The gating mechanism in pre_config_uboot_target__build_first_stage function works as intended to conditionally build/copy tiboot3.bin only when TIBOOT3_BOOTCONFIG is defined.
Applied to files:
lib/functions/compilation/uboot.sh
📚 Learning: 2025-06-22T00:45:26.182Z
Learnt from: djurny
Repo: armbian/build PR: 8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:45:26.182Z
Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
Applied to files:
lib/functions/compilation/uboot.shextensions/ccache-remote.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
lib/functions/compilation/uboot.shextensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
lib/functions/compilation/uboot.shextensions/ccache-remote.sh
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.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:
extensions/ccache-remote.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/ccache-remote.sh
🧬 Code graph analysis (1)
lib/functions/compilation/uboot.sh (1)
lib/functions/general/extensions.sh (1)
call_extension_method(38-60)
🔇 Additional comments (8)
extensions/ccache-remote.sh (6)
1-70: Excellent documentation and security awareness.The inline documentation is comprehensive, covering usage examples, server setup, client requirements, and fallback behavior. The prominent security warning about the insecure Redis configuration (lines 36-38) is particularly important and well-placed.
71-91: Robust stats querying with graceful fallback.The function correctly handles both redis-cli availability and basic connectivity checks via netcat. The 2-second timeouts prevent hanging, and the graceful degradation when redis-cli is unavailable is well-implemented.
93-125: LGTM: Host-side mDNS resolution and Docker integration.The function correctly resolves ccache.local on the host (since mDNS doesn't work inside Docker) and properly passes the Redis endpoint to Docker via DOCKER_EXTRA_ARGS. The respect for pre-configured CCACHE_REMOTE_STORAGE (lines 100-104) provides good flexibility.
Note: Using the
add_host_dependencies__*hook for non-dependency purposes was discussed in previous reviews and accepted as pragmatic given current framework limitations.
127-149: Statistics reporting looks good.The function correctly:
- Explicitly sets CCACHE_DIR when querying stats (line 133)
- Parses remote_storage_* metrics using standard text processing
- Handles division by zero (lines 142-144)
- Provides useful hit rate calculation
The parsing approach using
grepandcutis standard for ccache stats and acceptable here.
151-177: Configuration setup follows established patterns.The function correctly:
- Enables ccache via
declare -g USE_CCACHE=yes(internal Armbian variable)- Registers cleanup handler for final stats
- Handles both Docker (pre-configured) and native build scenarios
- Uses
export CCACHE_REMOTE_STORAGE=...for native builds (line 170) to ensure child processes can read it- Falls back gracefully when ccache.local is unresolvable
The distinction between
declare -gandexportis intentional and well-explained in the PR comments.
179-195: Hook implementations for kernel and U-Boot builds are correct.Both functions properly inject CCACHE_REMOTE_STORAGE into their respective make environments (
common_make_envsfor kernel,uboot_make_envsfor U-Boot). This is necessary because both build systems useenv -i, which clears all environment variables.The quoting is correct:
"CCACHE_REMOTE_STORAGE='${CCACHE_REMOTE_STORAGE}'"will properly pass the variable through the env -i barrier.lib/functions/compilation/uboot.sh (2)
243-249: Correct propagation of ccache directories.The new code properly adds CCACHE_DIR and CCACHE_TEMPDIR to the uboot_make_envs array when these variables are set. This is necessary because the U-Boot build uses
env -i(line 265), which clears all environment variables. The quoting is correct, and the conditional checks prevent adding empty values.This mirrors the approach used in kernel builds and integrates cleanly with the new ccache-remote extension.
255-260: Well-documented extension hook for U-Boot make environment.The new
uboot_make_confighook allows extensions to modify the U-Boot make environment before compilation. The documentation clearly explains:
- When the hook is called (right before make invocation)
- What can be modified (uboot_make_envs array)
- Use case (e.g., CCACHE_REMOTE_STORAGE)
This follows the established extension pattern and enables the ccache-remote extension to inject CCACHE_REMOTE_STORAGE via
uboot_make_config__add_ccache_remote_storage().
|
Great idea! |
|
please, add CCACHE_REMOTE_ONLY option to disable local ccache storage. |
github-actions
bot
added
size/large
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @extensions/ccache-remote.sh:
- Around line 173-195: In ccache_remote_show_final_stats the grep results
(read_hit/read_miss/write/error) can be empty strings causing a bash arithmetic
error when computing total and pct; fix by normalizing those variables to
numeric defaults before arithmetic (e.g. assign read_hit=${read_hit:-0}
read_miss=${read_miss:-0} write=${write:-0} error=${error:-0} or use parameter
expansion where used) so total=$((read_hit + read_miss)) and pct calculation
never operate on empty values; ensure CCACHE_DIR/stats_output handling remains
the same and keep display_alert calls unchanged.
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
1-91: Add ccache version validation when extension initializes.The documentation specifies ccache 4.4+ is required for remote storage support, but the extension doesn't validate this at initialization. If an older ccache version is installed, the remote storage configuration will silently fail to work without clear feedback to the user. Consider checking the ccache version in
extension_prepare_config__setup_remote_ccache()and displaying a warning or error if the version is incompatible.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
extensions/ccache-remote.sh
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.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:
extensions/ccache-remote.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-06-22T00:45:26.182Z
Learnt from: djurny
Repo: armbian/build PR: 8315
File: lib/functions/rootfs/distro-agnostic.sh:54-68
Timestamp: 2025-06-22T00:45:26.182Z
Learning: In Armbian build system, external command dependencies like envsubst are handled by adding the required packages (e.g., gettext for envsubst) to host_dependencies in adaptative_prepare_host_dependencies function in lib/functions/host/prepare-host.sh, rather than checking command availability at runtime.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.sh
🧬 Code graph analysis (1)
extensions/ccache-remote.sh (2)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)lib/functions/logging/traps.sh (1)
add_cleanup_handler(128-141)
🔇 Additional comments (6)
extensions/ccache-remote.sh (6)
92-109: LGTM!The CCACHE_PASSTHROUGH_VARS array is well-defined and includes all necessary ccache environment variables, including CCACHE_REMOTE_ONLY which addresses the feature request mentioned in the PR comments.
111-131: LGTM!The get_redis_stats function has proper timeout protection, graceful fallback handling, and appropriate error suppression. The function correctly handles cases where redis-cli is unavailable or the connection fails.
133-171: LGTM!The host-side mDNS resolution and Docker environment setup is well-implemented. The function correctly:
- Resolves ccache.local only when needed (not pre-configured)
- Displays Redis stats for visibility
- Propagates all relevant CCACHE_* variables to Docker
The use of add_host_dependencies hook for non-dependency tasks was discussed in past reviews and is acceptable.
197-221: LGTM!The configuration function correctly:
- Enables USE_CCACHE for the Armbian build framework
- Registers the cleanup handler for end-of-build stats
- Performs mDNS resolution (needed for native builds where the host hook doesn't run)
- Uses
exportfor CCACHE_REMOTE_STORAGE (required for child process visibility) anddeclare -gfor USE_CCACHE (internal Armbian variable)The distinction between
declare -gandexportwas clarified in the PR comments and is intentional.
223-234: LGTM!The kernel_make_config hook correctly propagates ccache variables to the kernel build environment. The format
${var}='${val}'with single quotes is appropriate for preventing shell expansion when these are passed to the make command.This addresses the challenge that kernel builds use
env -iwhich clears environment variables.
236-247: LGTM!The U-Boot hook mirrors the kernel implementation and correctly propagates ccache variables to the U-Boot build environment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/functions/host/docker.sh (1)
583-587: Consider initializing DOCKER_EXTRA_ARGS before the hook for clarity.The current logic correctly handles unset, empty, and populated arrays. However, explicitly initializing
DOCKER_EXTRA_ARGSas an empty array before calling the hook would make the intent clearer and follow defensive coding practices, similar to howDOCKER_ARGSis initialized at line 378.♻️ Suggested initialization
Add this declaration before the hook call:
+ declare -g -a DOCKER_EXTRA_ARGS=() + # Hook for extensions to add Docker arguments before launch call_extension_method "host_pre_docker_launch" <<- 'HOST_PRE_DOCKER_LAUNCH'
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
lib/functions/host/docker.sh
🧰 Additional context used
🧠 Learnings (6)
📚 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:
lib/functions/host/docker.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/host/docker.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/host/docker.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/host/docker.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
lib/functions/host/docker.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
lib/functions/host/docker.sh
🧬 Code graph analysis (1)
lib/functions/host/docker.sh (1)
lib/functions/general/extensions.sh (1)
call_extension_method(38-60)
🔇 Additional comments (1)
lib/functions/host/docker.sh (1)
572-581: Well-implemented extension hook with clear documentation.The
host_pre_docker_launchhook is properly placed and documented, allowing extensions to add Docker arguments just before container launch. The heredoc provides clear guidance on available variables and usage patterns, which will help extension authors use this hook correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @extensions/ccache-remote.sh:
- Around line 173-195: In ccache_remote_show_final_stats, the variables
read_hit/read_miss/write/error can be empty if grep finds nothing which breaks
the arithmetic; after extracting them (the assignments for read_hit, read_miss,
write, error) default each to 0 (e.g., read_hit=${read_hit:-0} etc.) or use the
defaults directly in the arithmetic/pct computation (use ${read_hit:-0} and
${read_miss:-0}) so total and pct calculations never receive empty strings and
won't trigger a bash arithmetic error under set -e.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
extensions/ccache-remote.shlib/functions/compilation/kernel-make.shlib/functions/compilation/uboot.shlib/functions/host/docker.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/functions/compilation/uboot.sh
- lib/functions/host/docker.sh
🧰 Additional context used
🧠 Learnings (8)
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.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:
extensions/ccache-remote.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
extensions/ccache-remote.shlib/functions/compilation/kernel-make.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.shlib/functions/compilation/kernel-make.sh
📚 Learning: 2025-09-22T21:52:01.225Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:194-201
Timestamp: 2025-09-22T21:52:01.225Z
Learning: The kernel_config_set_m function in lib/functions/compilation/armbian-kernel.sh preserves existing 'y' (built-in) settings and only sets options to 'm' (module) when they are not already built-in, achieving "prefer modules but allow built-ins" behavior.
Applied to files:
lib/functions/compilation/kernel-make.sh
🧬 Code graph analysis (2)
extensions/ccache-remote.sh (2)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)lib/functions/logging/traps.sh (1)
add_cleanup_handler(128-141)
lib/functions/compilation/kernel-make.sh (1)
lib/functions/general/extensions.sh (1)
call_extension_method(38-60)
🔇 Additional comments (8)
lib/functions/compilation/kernel-make.sh (1)
73-81: LGTM! Well-documented extension hook.The new
kernel_make_configextension hook is properly placed and documented. It allows extensions to customize kernel compilation environment variables and make parameters before the finalenv -iinvocation, which is essential for ccache-remote and similar extensions that need to propagate environment state through theenv -ibarrier.extensions/ccache-remote.sh (7)
1-91: Excellent documentation.The comprehensive documentation covering setup, usage, security considerations, and fallback behavior is very helpful. The explicit warning about the insecure Redis configuration (lines 57-59) is particularly important.
92-109: LGTM! Comprehensive passthrough variable list.The
CCACHE_PASSTHROUGH_VARSarray correctly lists all relevant ccache environment variables that need to propagate throughenv -ibarriers in kernel and U-Boot builds.
111-131: LGTM! Appropriate stats gathering for informational display.The function properly handles Redis stats collection with timeout protection and netcat fallback. Since this is purely informational (used only for
display_alert), the minor edge cases (e.g., unexpectedDBSIZEformat) don't impact functionality.
133-171: LGTM! Proper Docker environment propagation.The function correctly resolves mDNS on the host (where it works) before Docker launch, queries Redis stats for visibility, and properly propagates all set
CCACHE_*variables to the Docker container viaDOCKER_EXTRA_ARGS.
197-221: LGTM! Proper configuration and cleanup registration.The function correctly enables ccache, registers the stats cleanup handler, and gracefully falls back to local-only cache when remote storage is unavailable. The use of
declare -gforUSE_CCACHE(line 200) vsexportforCCACHE_REMOTE_STORAGE(line 211) is appropriate—internal config variables don't need subprocess visibility.
223-234: LGTM! Correct kernel make environment propagation.The hook properly propagates all set ccache environment variables to the kernel build's
common_make_envsarray, which is essential becausekernel-make.shusesenv -ito create a clean environment.
236-247: LGTM! Correct U-Boot make environment propagation.The hook mirrors the kernel implementation but correctly targets
uboot_make_envs, ensuring ccache variables propagate through U-Boot'senv -ibarrier as well.
We use prepared dockerfile with fresh enough version, then we don't care. |
ok. accepted. |
done |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @extensions/ccache-remote.sh:
- Around line 173-195: In ccache_remote_show_final_stats the arithmetic
total=$((read_hit + read_miss)) fails when grep returns empty strings; ensure
read_hit/read_miss/write/error are normalized to numeric defaults before any
arithmetic by: after the grep/cut lines in function
ccache_remote_show_final_stats (variables read_hit, read_miss, write, error)
coerce them to digits and set defaults (e.g. strip non-digits then use
${var:-0}) or explicitly assign read_hit=${read_hit:-0} and
read_miss=${read_miss:-0} (and similarly for write/error) so total=$((read_hit +
read_miss)) and pct calculation never receive empty strings and the cleanup
handler cannot crash.
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
223-234: Potential quoting issue with special characters in values.Line 230 constructs
"${var}='${val}'". Ifvalcontains single quotes, the resulting string will be malformed. While typical ccache values (paths, sizes, comma-separated options) rarely contain quotes, edge cases likeCCACHE_LOGFILEwith unusual paths could break.♻️ Safer quoting approach
for var in "${CCACHE_PASSTHROUGH_VARS[@]}"; do val="${!var}" if [[ -n "${val}" ]]; then - common_make_envs+=("${var}='${val}'") + common_make_envs+=("${var}=${val@Q}") display_alert "Kernel make: ${var}" "${val}" "debug" fi doneThe
${val@Q}parameter expansion (Bash 4.4+) properly quotes the value for shell reuse.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
extensions/ccache-remote.shlib/functions/compilation/uboot.shlib/functions/host/docker.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/functions/compilation/uboot.sh
- lib/functions/host/docker.sh
🧰 Additional context used
🧠 Learnings (8)
📓 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.
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.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:
extensions/ccache-remote.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.sh
🧬 Code graph analysis (1)
extensions/ccache-remote.sh (2)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)lib/functions/logging/traps.sh (1)
add_cleanup_handler(128-141)
🔇 Additional comments (6)
extensions/ccache-remote.sh (6)
1-91: Well-documented extension header.The documentation is comprehensive, covering usage examples, supported environment variables, server/client setup, security warnings, and fallback behavior. The security warning about the insecure Redis configuration (lines 57-59) is appropriately prominent.
92-109: LGTM!The global array declaration is correct, and the passthrough variables align with ccache's documented configuration options.
111-131: LGTM with minor observation.The function correctly uses timeouts to prevent blocking and handles missing tools gracefully. The fallback to netcat for basic connectivity is a good approach.
Minor: At line 122,
${keys:-0}is redundant since the block only executes when$keysis non-empty (line 121 condition). This doesn't affect functionality.
133-171: LGTM!The host-side mDNS resolution before Docker launch is correctly implemented. The function properly:
- Checks for pre-configured
CCACHE_REMOTE_STORAGEbefore attempting discovery- Uses
getent hostsfor mDNS resolution (works with both systemd-resolved and avahi)- Passes environment variables to Docker via
DOCKER_EXTRA_ARGS- Provides appropriate debug logging when discovery fails
197-221: LGTM!The function correctly:
- Enables ccache via the internal
USE_CCACHEvariable- Registers the cleanup handler for end-of-build stats
- Handles native builds (non-Docker) by attempting mDNS resolution if
CCACHE_REMOTE_STORAGEwasn't passed from the hostThe duplication of mDNS resolution logic between this function and
host_pre_docker_launch__setup_remote_ccacheis intentional to support both Docker and native build paths.
236-247: Same quoting consideration applies.This function mirrors
kernel_make_config__add_ccache_remote_storagewith the same quoting pattern. The same optional refactor using${val@Q}would apply here for consistency.The parallel structure targeting
uboot_make_envsvscommon_make_envsis appropriate given the different build contexts.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
extensions/ccache-remote.sh (1)
133-171: Verify DOCKER_EXTRA_ARGS initialization.The logic is correct, but based on the past review discussion, you accepted a suggestion to initialize
DOCKER_EXTRA_ARGSbefore using it in thehost_pre_docker_launchhook. Line 167 usesdeclare -g -a DOCKER_EXTRA_ARGS+=(...)which will work even if the array doesn't exist, but explicit initialization would be more defensive.Consider adding at the beginning of this function (after line 138):
declare -g -a DOCKER_EXTRA_ARGS=("${DOCKER_EXTRA_ARGS[@]}")This ensures the array exists without losing any pre-existing values.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
extensions/ccache-remote.sh
🧰 Additional context used
🧠 Learnings (8)
📓 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.
📚 Learning: 2025-12-16T12:22:20.156Z
Learnt from: tabrisnet
Repo: armbian/build PR: 9085
File: lib/functions/rootfs/rootfs-create.sh:303-306
Timestamp: 2025-12-16T12:22:20.156Z
Learning: The post_debootstrap_customize hook concept in lib/functions/rootfs/rootfs-create.sh has been tested in PR #9000. The hook placement after package installations (including desktop packages) and before cleanup operations (autoremove, qemu undeploy) is validated as appropriate for rootfs customization.
Applied to files:
extensions/ccache-remote.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:
extensions/ccache-remote.sh
📚 Learning: 2025-09-24T09:54:07.968Z
Learnt from: amazingfate
Repo: armbian/build PR: 8668
File: extensions/ufs.sh:2-2
Timestamp: 2025-09-24T09:54:07.968Z
Learning: In Armbian's build system, DOCKER_ARMBIAN_BASE_IMAGE must be set globally at file load time in extensions, not inside extension_prepare_config functions, because docker operations happen early in the build process and would default to ubuntu:noble if not set before docker_cli_prepare() runs.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-04-28T08:27:26.890Z
Learnt from: leggewie
Repo: armbian/build PR: 8133
File: extensions/apa.sh:1-2
Timestamp: 2025-04-28T08:27:26.890Z
Learning: In the Armbian build system, extension scripts in the `extensions/` directory contain hook functions and are meant to be sourced, not executed directly. These scripts don't require a shebang or `set -euo pipefail`.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-06-14T05:53:10.627Z
Learnt from: EvilOlaf
Repo: armbian/build PR: 8298
File: extensions/https_proxy.sh:1-9
Timestamp: 2025-06-14T05:53:10.627Z
Learning: Armbian build framework extensions are NOT designed to run standalone but are called by the supervisor. The framework provides environment variables like USERPATCHES_PATH and SDCARD, and commands are executed with protection via run_host_command_logged and chroot_sdcard functions that use bash -e -o pipefail.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-09-22T21:59:36.334Z
Learnt from: tabrisnet
Repo: armbian/build PR: 8661
File: lib/functions/compilation/armbian-kernel.sh:200-200
Timestamp: 2025-09-22T21:59:36.334Z
Learning: Functions named with the pattern `armbian_kernel_config__*` in lib/functions/compilation/armbian-kernel.sh are automatically discovered and invoked by Armbian's extension system via metaprogramming using `compgen -A function`, without requiring explicit registration or calls.
Applied to files:
extensions/ccache-remote.sh
📚 Learning: 2025-12-18T23:40:41.627Z
Learnt from: rpardini
Repo: armbian/build PR: 9101
File: lib/functions/image/compress-checksum.sh:44-48
Timestamp: 2025-12-18T23:40:41.627Z
Learning: In the Armbian build framework, scripts run with set -e, so a non-zero exit status will abort the script. During reviews, assume failures will stop execution unless explicitly handled. Verify that commands that must fail are checked, errors are propagated, and any critical steps have proper error handling (e.g., using pipefail where appropriate, checking exit codes, and not masking failures).
Applied to files:
extensions/ccache-remote.sh
🧬 Code graph analysis (1)
extensions/ccache-remote.sh (2)
lib/functions/logging/display-alert.sh (1)
display_alert(10-227)lib/functions/logging/traps.sh (1)
add_cleanup_handler(128-141)
🔇 Additional comments (6)
extensions/ccache-remote.sh (6)
1-91: LGTM! Excellent documentation.The documentation is comprehensive and covers all key aspects: usage examples, Avahi/mDNS setup, security warnings, Redis configuration, fallback behavior, and cache sharing requirements. The security warning about the insecure Redis configuration is particularly important and well-placed.
92-109: LGTM! Proper array declaration.The global array declaration and variable list are correct and match the documented environment variables.
111-131: LGTM! Good error handling and fallback strategy.The function properly handles missing
redis-cliwith a netcat fallback and safely checks for empty results before using them.
197-221: LGTM! Correct variable scoping and cleanup handler registration.The use of
declare -g USE_CCACHE=yes(line 200) for the internal Armbian variable andexport CCACHE_REMOTE_STORAGE(line 211) for the ccache subprocess is intentionally different and correct, as explained in the PR comments. The cleanup handler registration and fallback logic are properly implemented.
223-234: LGTM! Correct environment variable propagation to kernel make.The function correctly propagates ccache variables to the kernel make environment, properly handling the
env -iclearing. The${val@Q}quoting is appropriate for safe shell variable expansion.
236-247: LGTM! Correct environment variable propagation to U-Boot make.The function mirrors the kernel make hook and correctly propagates ccache variables to the U-Boot make environment.
The initialization is already handled in docker.sh:573 where we added declare -g -a DOCKER_EXTRA_ARGS=() before calling the host_pre_docker_launch hook. This ensures the array exists before any extension accesses it. |
is processed, allowing extensions to add Docker arguments with a more descriptive hook name than add_host_dependencies.
Enables ccache with remote Redis storage for sharing compilation cache across build hosts. Features: - Auto-discovery via Avahi/mDNS (ccache.local hostname) - Explicit Redis server configuration via CCACHE_REMOTE_STORAGE - Build statistics display at end of build (hit/miss/error rates) - Support for both Docker and native builds - Hooks for kernel and u-boot compilation environments Documentation includes server setup instructions with security warnings, client mDNS configuration, and cache sharing requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
U-Boot build uses `env -i` which clears all environment variables. CCACHE_DIR and CCACHE_TEMPDIR were not explicitly passed to make, unlike kernel build (kernel-make.sh). This caused ccache to use default directory instead of configured Armbian one, breaking cache statistics and shared cache functionality. Changes: - Add CCACHE_DIR and CCACHE_TEMPDIR to uboot_make_envs - Add uboot_make_config hook for extensions (similar to kernel_make_config), allowing modification of environment variables before compilation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
3 participants
Add ccache-remote extension for distributed compilation caching via Redis
Description
This PR adds support for sharing ccache compilation cache between multiple build machines via a Redis server.
Motivation
When building Armbian on multiple machines (e.g., in CI or on a local developer network), each machine maintains its own ccache. This leads to:
Solution
The new ccache-remote extension enables shared ccache storage via a Redis server. It supports:
Changes
New files:
Modified files:
Usage
With Avahi auto-discovery (requires server setup)
With explicit Redis server
Technical Details
Testing
Checklist:
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.