Extension: ccache-remote — shared compilation cache via Redis/HTTP #9369
Conversation
iav
requested review from
a team,
EvilOlaf,
TRSx80,
igorpecovnik and
littlecxm
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
📝 WalkthroughWalkthroughAdds a new ccache-remote extension implementing remote ccache storage discovery and integration (Redis and HTTP/WebDAV), server setup docs and service templates (Redis/nginx/Avahi/systemd), compilation/ccache stat helpers, Docker pre-launch hook, and new kernel/uboot make-environment hooks. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as Build Client
participant Discovery as Discovery (Avahi / DNS‑SRV / mDNS)
participant Redis as Redis Backend
participant HTTP as HTTP/WebDAV Backend
participant Config as CCACHE Configurator
Client->>Discovery: Discover _ccache._tcp services
alt Services found
Discovery->>Discovery: Prioritize Redis over HTTP
alt Redis available
Discovery->>Redis: Connect (AUTH/PING)
Redis-->>Discovery: OK
Discovery->>Config: Configure redis:// storage
else HTTP available
Discovery->>HTTP: HEAD/OPTIONS /ccache/
HTTP-->>Discovery: OK
Discovery->>Config: Configure http:// storage
end
else No discovery
Client->>Config: Use explicit CCACHE_REMOTE_STORAGE
end
Config-->>Client: Remote storage configured
sequenceDiagram
participant Build as Build System
participant PreDocker as host_pre_docker_launch Hook
participant Docker as Docker CLI/Daemon
participant Container as Build Container
participant StatsHook as ccache_post_compilation Hook
Build->>PreDocker: Invoke host_pre_docker_launch
PreDocker->>PreDocker: Resolve hostnames, rewrite loopback, append DOCKER_EXTRA_ARGS
PreDocker->>Docker: Launch with merged DOCKER_ARGS
Docker->>Container: Start container with CCACHE envs
Container->>Container: Compile using remote cache
Container-->>StatsHook: Trigger post-compilation hook
StatsHook->>Build: Display remote ccache stats
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
bot
added
size/large
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@extensions/ccache-remote/README.server-setup.md`:
- Around line 116-125: The fenced code blocks showing DNS records for ccache
(the two blocks containing "_ccache._tcp.example.com. SRV 0 0 6379 ..." and
"_ccache._tcp.example.com. SRV 0 0 8088 ...") need a language specifier to
satisfy MD040; update each triple-backtick fence to include a language such as
"text" or "dns" (e.g., ```text) so both code fences that contain the SRV/TXT
examples are annotated.
- Around line 39-40: Replace the outdated Redis security URL in the README entry
that currently points to "https://redis.io/docs/management/security/" with the
updated documentation URL
"https://redis.io/docs/latest/operate/oss_and_stack/management/security/";
locate the occurrence of the old URL in the README.server-setup.md text and
update the string so the link points to the new location.
In `@lib/functions/host/host-release.sh`:
- Line 36: The HOSTRELEASE allowlist in host-release.sh was temporarily expanded
to include "questing"; revert that change by removing "questing" from the string
in the conditional that checks HOSTRELEASE (the line containing the if [[ -z
$HOSTRELEASE || "bookworm trixie forky sid jammy kinetic lunar vanessa vera
victoria virginia wilma mantic noble resolute questing" != *"$HOSTRELEASE"* ]];
then) so the allowlist returns to its original set of releases before merging.
🧹 Nitpick comments (4)
extensions/ccache-remote/misc/nginx/ccache-webdav.conf (1)
14-26: Ensurengx_http_dav_moduleis available.The
dav_methodsandcreate_full_put_pathdirectives require thengx_http_dav_module, which is not included in all nginx packages (e.g., the defaultnginx-coreon Debian/Ubuntu doesn't include it —nginx-fullornginx-extrasis needed). Consider adding a note in the comments about the required nginx package/module.Suggested comment addition
# WARNING: No authentication configured. # Use ONLY in a fully trusted private network. # For auth, add auth_basic directives. See nginx WebDAV documentation. # Note: ccache does not support HTTPS directly. Use a reverse proxy for TLS. +# Requires: nginx with http_dav_module (e.g. nginx-full or nginx-extras on Debian/Ubuntu)lib/functions/compilation/kernel-make.sh (1)
77-86: Newkernel_make_confighook: verify ordering intent with existingcustom_kernel_make_params.Both
kernel_make_config(Line 77) andcustom_kernel_make_params(Line 88) allow modifying the same arrays (common_make_envs,common_make_params_quoted). The new hook runs first, socustom_kernel_make_paramscan override anything set bykernel_make_config. This ordering seems intentional (generic extension config → custom overrides), but consider adding a brief inline comment clarifying the precedence to avoid future confusion.extensions/ccache-remote/ccache-remote.sh (2)
345-356: Loopback URL rewrite uses global string substitution — safe in practice but slightly fragile.
${CCACHE_REMOTE_STORAGE//localhost/host.docker.internal}replaces every occurrence oflocalhostin the string, not just the host portion. Iflocalhostappeared in a password or path, it would be corrupted. The detection regex (lines 347–349) correctly scopes to the host position, but the replacement doesn't.This is very unlikely to be a real problem, but a targeted substitution (e.g., using
sedon the host component, or matching the scheme-prefix in the replacement) would be more precise.
126-138: Misplaced doc comment — describesccache_discover_remote_storage, notccache_format_host_port.The comment block at lines 126–129 documents the discovery function but immediately precedes the
ccache_format_host_porthelper. Move it aboveccache_discover_remote_storage(line 140) to avoid confusion.Proposed fix
-# Discover ccache remote storage via DNS-SD (mDNS/Avahi) or DNS SRV records. -# Looks for _ccache._tcp services with TXT records: type=http|redis, path=/... -# Prefers Redis over HTTP when multiple services are found. -# Sets CCACHE_REMOTE_STORAGE on success, returns 1 if nothing found. +# Format host:port, wrapping IPv6 addresses in brackets for URL compatibility (RFC 2732) function ccache_format_host_port() { local host="$1" port="$2" - # Wrap IPv6 addresses in brackets for URL compatibility (RFC 2732) if [[ "${host}" == *:* ]]; then echo "[${host}]:${port}" else echo "${host}:${port}" fi } +# Discover ccache remote storage via DNS-SD (mDNS/Avahi) or DNS SRV records. +# Looks for _ccache._tcp services with TXT records: type=http|redis, path=/... +# Prefers Redis over HTTP when multiple services are found. +# Sets CCACHE_REMOTE_STORAGE on success, returns 1 if nothing found. function ccache_discover_remote_storage() {
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@extensions/ccache-remote/ccache-remote.sh`:
- Around line 345-358: The loopback detection on CCACHE_REMOTE_STORAGE fails
when the URL contains userinfo (username:password@) because the regex expects
the host immediately after "://"; fix by extracting the authority host portion
before checking (e.g., derive host_part via parameter expansion like
host_part="${CCACHE_REMOTE_STORAGE##*@}" or by stripping up to "://"), run the
loopback regex checks against that host_part (checking for localhost, 127.0.0.1,
or [::1]) and if matched, rewrite only the host portion (reconstruct
CCACHE_REMOTE_STORAGE by replacing the host substring after the last '@' or
after '://') before adding DOCKER_EXTRA_ARGS and calling
display_alert/ccache_mask_storage_url so passwords/userinfo are not used for
detection or blind global replacement.
🧹 Nitpick comments (1)
extensions/ccache-remote/ccache-remote.sh (1)
140-224: DNS SRV discovery only processes the first record; no priority/weight sorting.Line 187 reads a single line from
dig +short SRVoutput. If multiple SRV records are published (e.g., primary and failover), only the first line is used regardless of priority/weight fields. This is fine as a pragmatic first implementation, but worth documenting.Also, the default
svc_type="redis"on Line 191 when no TXT record is present is a reasonable convention but makes HTTP-only SRV setups silently fail if the TXT record is missing. Consider adding a debug-level alert when falling back to the default type.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@extensions/ccache-remote/ccache-remote.sh`:
- Around line 168-201: The loop currently sets globals CCACHE_REMOTE_HOST and
CCACHE_REMOTE_HOST_IP on every iteration which can point to the wrong service
after the loop chooses redis_url or http_url; instead, record host/IP per
service type during the loop (e.g., local redis_host/redis_ip and
http_host/http_ip when you set redis_url or http_url) and only set the global
CCACHE_REMOTE_HOST and CCACHE_REMOTE_HOST_IP after deciding the winning URL
(when you export CCACHE_REMOTE_STORAGE), assigning them from the corresponding
per-service variables so Docker --add-host uses the host for the actually
selected service.
- Around line 140-143: The userinfo stripping in the block manipulating
after_scheme uses a non-greedy removal ("${after_scheme#*@}") which drops up to
the first '@' and can mis-handle userinfo containing '@'; change the expression
to the greedy form ("${after_scheme##*@}") to strip through the last '@'
consistently with ccache_mask_storage_url and ensure the host is extracted
correctly; update the line that currently sets after_scheme to use
after_scheme="${after_scheme##*@}" (reference variable after_scheme and the
ccache_mask_storage_url behavior).
8858e87 to
1300bf6
Compare
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@extensions/ccache-remote/misc/redis/redis-ccache.conf`:
- Line 17: The active ACL entry "user default on >CHANGE_ME sanitize-payload ~*
&* +@all" exposes a placeholder password; either comment out this line or remove
it so the default user isn't created with a well-known password, or replace
>CHANGE_ME with a secure secret from your secrets management; update the "user
default ..." line in redis-ccache.conf (or add a comment above it) to ensure a
copy-paste deployment cannot enable this placeholder credential.
In `@extensions/ccache-remote/README.server-setup.md`:
- Around line 10-11: The README instruction currently directs copying
misc/redis/redis-ccache.conf to /etc/redis/redis.conf which would overwrite the
stock Redis config; update the wording to warn that this replaces the default
file and instead recommend adding the ccache config via an include or copying to
a separate file (for example /etc/redis/redis-ccache.conf) and referencing it
from the main config, or explicitly instruct how to merge the settings so users
don’t lose default directives; mention the filenames
misc/redis/redis-ccache.conf and /etc/redis/redis.conf and suggest the
include-based approach as the preferred safe option.
In `@lib/functions/compilation/kernel-make.sh`:
- Around line 77-88: The heredoc passed to call_extension_method
"kernel_make_config" incorrectly documents a read-only variable named
`toolchain`; edit the KERNEL_MAKE_CONFIG block to remove `toolchain` from the
"Available read-only variables" list (leaving KERNEL_COMPILER, ARCHITECTURE,
BRANCH, LINUXFAMILY intact) so extension authors aren't misled; this change is
within the kernel_make_config heredoc that mentions common_make_envs[@] and
common_make_params_quoted[@].
🧹 Nitpick comments (3)
extensions/ccache-remote/misc/systemd/ccache-avahi-redis.service (1)
1-4: Consider addingRequires=avahi-daemon.serviceto ensure Avahi is running.
After=only orders startup but doesn't pull in the dependency. Ifavahi-daemonisn't active,avahi-publish-servicewill fail repeatedly (caught byRestart=on-failure, but wasteful). AddingRequires=makes the dependency explicit.Suggested fix
[Unit] Description=Publish ccache Redis service via Avahi DNS-SD +Requires=avahi-daemon.service After=avahi-daemon.service redis-server.service BindsTo=redis-server.serviceextensions/ccache-remote/README.server-setup.md (1)
74-98: WebDAV section lacks any authentication guidance beyond the warning.The Redis section provides detailed auth instructions, but the WebDAV section only has a "WARNING: No authentication configured" note with no follow-up on how to add basic auth or IP restrictions in nginx. A brief pointer (e.g.,
auth_basicorallow/denydirectives) would make the guide more consistent.lib/functions/compilation/ccache.sh (1)
15-15: Grep pattern may match unintended field name prefixes.
grep "^${field}"will also match fields that start with the same prefix (e.g., a hypotheticaldirect_cache_hit_preprocesswhen searching fordirect_cache_hit). Anchoring with a tab after the field name would be more precise.♻️ Suggested fix
- val=$(echo "$stats_output" | grep "^${field}" | cut -f2 || true) + val=$(echo "$stats_output" | grep -P "^${field}\t" | cut -f2 || true)
iav
force-pushed
the
enh/ccache_on_network
branch
from
1300bf6 to
b230297
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
iav
force-pushed
the
enh/ccache_on_network
branch
2 times, most recently
from
310c8eb to
15fc317
Compare
iav
force-pushed
the
enh/ccache_on_network
branch
2 times, most recently
from
0f5cab4 to
cf63de1
Compare
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 <noreply@anthropic.com>
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 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ndler Show remote ccache stats after each compilation (kernel, uboot) via hook, instead of once at the end via cleanup handler. Stats now shown even on build failure.
- uboot.sh: clarify ARMBIAN=foe workaround for dual-compiler scenario - ccache-remote.sh: document that CCACHE_REDIS_CONNECT_TIMEOUT must be set before extension loads Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Mask CCACHE_REMOTE_STORAGE when emitting Docker env debug logs.
…sthrough loops Extract ccache_inject_envs() helper to deduplicate identical passthrough loops in kernel and uboot make config hooks. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> ccache-remote: rename functions to follow project naming conventions Rename get_redis_stats and mask_storage_url to ccache_get_redis_stats and ccache_mask_storage_url to follow project naming conventions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> ccache-remote: mask credentials in debug log output for passthrough loops Mask CCACHE_REMOTE_STORAGE value through ccache_mask_storage_url() before logging in both Docker env and make env passthrough loops to avoid leaking credentials into build logs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add separate insecure config example for trusted networks. Recommend URL-safe hex passwords and update setup docs. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
iav
force-pushed
the
enh/ccache_on_network
branch
from
cf63de1 to
e4eeb84
Compare
1 participant
Summary
Add
ccache-remoteextension that enables sharing compilation cache across build hosts via Redis or HTTP/WebDAV backends (ccache 4.4+). This significantly speeds up repeated builds on multiple machines or CI runners by reusing compiled objects from a shared cache server.Features
ccache.local)CCACHE_REMOTE_ONLY=yes, keeping only network storage to save local disk spaceCore framework changes
lib/functions/compilation/ccache.sh— addccache_post_compilationhook, stats display helpers, cleanup handler integrationlib/functions/compilation/kernel-make.sh— addkernel_make_confighook for extensions to inject env varslib/functions/compilation/uboot.sh— fix ccache environment passthrough throughenv -i, adduboot_make_confighooklib/functions/host/docker.sh— adddocker_pre_extra_argshook for extensions to add Docker argumentslib/functions/configuration/compilation-config.sh— whitelist ccache-related env varsExtension layout (
extensions/ccache-remote/)ccache-remote.sh— main extension (442 lines)README.server-setup.md— server setup documentationmisc/— ready-to-use config files: Redis, nginx WebDAV, Avahi DNS-SD services, systemd unitsUsage examples
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation