refactor(compilation): extract ccache as extension (prep) + add compile_wrapper hooks#122
refactor(compilation): extract ccache as extension (prep) + add compile_wrapper hooks#122iav wants to merge 11 commits into
Conversation
The variable has been declared but never assigned or used since 8278dc5 (2023-06-20, 'allwinner: Enable crust compilation'), which copy-pasted the binutils_version + binutils_flags skeleton from atf.sh but did not bring along the gcc/ld probe block that conditionally populates the flag, nor the TF_LDFLAGS-style use site. Crust is or1k-elf firmware where --no-warn-rwx-segment is not relevant, so the absence of the probe is by design; the leftover empty declaration is just dead. Removes a pre-existing SC2034 'appears unused' warning that surfaced when the file is touched by other refactors. Assisted-by: Claude:claude-opus-4.7
ATF hardcoded 'ccache ${ATF_COMPILER}' in CROSS_COMPILE/CC, ignoring
USE_CCACHE/PRIVATE_CCACHE toggles. Other compile paths (kernel-make.sh:60,
crust.sh:55,59) already use ${CCACHE} which is set to 'ccache' or '' by
prepare_compilation_vars in compilation-config.sh based on the toggle.
This also makes ATF compile consistent with the upcoming
extension-based compile-cache wrapper architecture, where extensions
(ccache, sccache, …) set ${CCACHE} to their respective binary.
Assisted-by: Claude:claude-opus-4.7
…n hooks Add call_extension_method hooks symmetric to the existing kernel_make_config / uboot_make_config: - atf_make_config in lib/functions/compilation/atf.sh - crust_make_config in lib/functions/compilation/crust.sh Both run right before the make invocation and allow extensions to modify environment and parameters (e.g., CCACHE_DIR, CCACHE_REMOTE_STORAGE, SCCACHE_DIR, SCCACHE_BUCKET, distcc envs). Symmetric to the existing kernel_make_config docstring. Prerequisite for the upcoming extension-based compile-cache wrapper architecture (ccache, sccache, …) where the wrap is set up via these hooks rather than via prepare_compilation_vars in core. Assisted-by: Claude:claude-opus-4.7
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 59c1420d7e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…e/post hooks
Introduce a generic compile-time wrapper independent of any specific
cache backend (ccache, sccache, distcc, …). The wrapper:
- Registers a cleanup handler for the post hook FIRST, so that even
if a pre hook starts a long-lived helper (sccache server, …) and
a later pre hook fails or the user interrupts mid-pre, the post
hook still fires for backend cleanup.
- Calls the 'compile_wrapper_pre' extension hook (zero-stats, start
helper servers, validate remote backend reachability, …)
- Runs the wrapped command
- Removes the cleanup handler and calls 'compile_wrapper_post'
explicitly on the success path
Default behavior with no extensions registered: passthrough — the
command runs unchanged. This makes the wrapper safe to drop in
everywhere a compilation invocation lives, without committing to any
specific cache backend.
A follow-up commit adds extensions/ccache.sh which implements both
hooks, then migrates the kernel/uboot/atf call sites from the legacy
do_with_ccache_statistics to do_with_compile_wrapper.
Assisted-by: Claude:claude-opus-4.7
…er + make_config hooks
Move the ccache compile-cache backend out of core into an opt-in
extension. The extension implements:
- extension_prepare_config__ccache
* defaults for CCACHE_DIR (private mode) and CCACHE_UMASK (shared)
* mutex check against other compile-cache extensions
- kernel_make_config__ccache / uboot_make_config__ccache
/ atf_make_config__ccache / crust_make_config__ccache
* set ${CCACHE}=ccache and prepend /usr/lib/ccache to PATH
- compile_wrapper_pre__ccache
* ccache --zero-stats, optional SHOW_CCACHE pre-stats
- compile_wrapper_post__ccache
* stats summary, optional SHOW_CCACHE size-diff,
invokes the legacy 'ccache_post_compilation' extension hook
for backward compatibility with 3rd-party extensions
Also adds an auto-enable shim in compilation-config.sh:
when USE_CCACHE=yes or PRIVATE_CCACHE=yes and neither 'ccache' nor
a competing compile-cache extension is already in ENABLE_EXTENSIONS,
'ccache' is appended. This preserves the existing user-facing toggle
with zero deprecation churn.
At this point both code paths are active for USE_CCACHE=yes users:
the legacy prepare_compilation_vars ccache block AND the new
extension. Their env-setup operations are idempotent (same ${CCACHE}
value, same PATH prefix). A follow-up commit migrates the call
sites from do_with_ccache_statistics to do_with_compile_wrapper,
and a later commit removes the legacy setup once the extension is
the sole owner.
Assisted-by: Claude:claude-opus-4.7
…pile_wrapper Switch the three existing do_with_ccache_statistics call sites (kernel.sh:259,263 and uboot.sh:273) to the backend-agnostic do_with_compile_wrapper introduced earlier. The legacy function remains in place for any out-of-tree consumers; it will be removed in a later commit once the ccache extension is the sole owner. No behavior change: do_with_compile_wrapper fires compile_wrapper_pre / compile_wrapper_post, which the auto-enabled extensions/ccache.sh implements with the same zero-stats / show-stats logic. Assisted-by: Claude:claude-opus-4.7
With the ccache extension and the auto-enable shim in place, the
legacy in-core ccache code is now redundant:
- extensions/ccache.sh owns env setup (${CCACHE}, PATH prepend,
CCACHE_DIR, CCACHE_UMASK) via the extension_prepare_config and
*_make_config hooks.
- extensions/ccache.sh owns stats reporting via the compile_wrapper_*
hooks.
- The shim in main-config.sh auto-enables the extension whenever
USE_CCACHE=yes / PRIVATE_CCACHE=yes is set, preserving the legacy
user-facing toggle.
This commit removes:
- lib/functions/compilation/ccache.sh entirely (~110 LOC).
do_with_ccache_statistics had no remaining callers after the
previous commit migrated them to do_with_compile_wrapper.
The ccache_get_stat / ccache_hit_pct / ccache_show_compilation_stats
helpers live in extensions/ccache.sh now (as _ext_ccache_get_stat
etc.) and the legacy 'ccache_post_compilation' extension hook is
invoked from compile_wrapper_post__ccache to preserve backward
compatibility with 3rd-party extensions that listened to it.
- The ccache block in lib/functions/configuration/compilation-config.sh
(prepare_compilation_vars), which exported ${CCACHE}, PATH,
CCACHE_DIR, CCACHE_UMASK. The extension does the same via its
extension_prepare_config__ccache and *_make_config hooks.
End state: ccache compile-cache integration lives entirely in
extensions/ccache.sh; core has no cache-backend awareness. This
clears the way for parallel cache backends (sccache, …) as new
extensions without further core changes.
Assisted-by: Claude:claude-opus-4.7
iav
force-pushed
the
refactor/ccache-as-extension
branch
from
59c1420 to
29a4bb9
Compare
|
@chatgpt review Sorry for the wrong mention earlier — please re-verify on head 29a4bb9. Variant B rewrite addresses both prior P2 findings:
Four open architectural questions are in the PR description above. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29a4bb93be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y helper names; auto-enable on ccache-remote Addresses 1 P1 + 5 P2 from codex review on head 29a4bb9. Order-independence (P1 + P2 'defer private cache setup'): All CCACHE_DIR / CCACHE_UMASK / PATH / ${CCACHE} setup moves out of extension_prepare_config__ccache (which runs at config-prepare phase, ordered alphabetically with other extensions' prepare_config hooks) into the four *_make_config__ccache hooks (kernel / uboot / atf / crust). By the time those fire, every extension's prepare_config has finished, so PRIVATE_CCACHE set by extensions/ccache-remote/ is visible to ccache as it should be. extension_prepare_config__ccache now only enforces mutex against other compile-cache extensions (sccache, …). ccache-remote awareness (P1): The auto-enable shim in main-config.sh also adds 'ccache' to ENABLE_EXTENSIONS when 'ccache-remote' is requested. ccache-remote sets USE_CCACHE=yes / PRIVATE_CCACHE=yes inside its own extension_prepare_config (too late for our config-phase shim), so the shim has to recognise the dependency by extension name. Export CCACHE_DIR / CCACHE_UMASK (P2 'export private CCACHE_DIR' + P2 'export shared-cache umask'): Use 'export' instead of 'declare -g'. The child shells spawned by run_host_command_logged inherit env, not bash globals; ccache reads CCACHE_DIR / CCACHE_UMASK from env. Plain declare-g left both at defaults, breaking the private cache path and the multi-user shared cache umask. PATH in-place (P2 'preserve kernel make PATH prefix'): kernel-make.sh:23 prepends ${PYTHON3_INFO[USERBASE]}/bin to PATH in common_make_envs. Our hook was appending a second 'PATH=' entry; env -i keeps only the last one, dropping the Python tools needed for dtb checks. Now we walk common_make_envs, find the existing PATH entry, and prepend /usr/lib/ccache to it; if none exists yet, we add one. Legacy helper names (P2 'keep legacy ccache helpers for remote stats'): extensions/ccache-remote/ccache-remote.sh's ccache_post_compilation__show_remote_stats hook calls ccache_get_stat and ccache_hit_pct. Restore the original names (drop the _ext_ prefix) so the 3rd-party hook keeps working. Assisted-by: Claude:claude-opus-4.7
|
@codex review Commit 7 (head P1 'auto-enable after remote': shim in main-config.sh now also triggers on P2 'keep legacy ccache helpers': P2 'export private CCACHE_DIR' / 'export shared-cache umask': P2 'preserve kernel make PATH prefix': P2 'defer private cache setup until remote config runs': All env wiring (CCACHE_DIR, CCACHE_UMASK, PATH, ${CCACHE}) moved out of Ordering invariant is now documented in the file header. |
|
@codex review (Correcting the mention handle — |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05bf8ee571
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…vars (right phase)
Earlier commit 7 moved env wiring into the per-artifact `*_make_config`
hooks, but those fire AFTER `run_kernel_make_internal` has already
expanded `${CCACHE}` into common_make_params_quoted (kernel-make.sh:60).
With CCACHE empty at that point, `make` got CROSS_COMPILE without
the ccache prefix — defeating the wrapper entirely (codex P1).
`prepare_compilation_vars` (lib/functions/configuration/compilation-config.sh)
is the architecturally correct phase: it runs late in default_build_start
(start-end.sh:20), after every `extension_prepare_config_*` (so
PRIVATE_CCACHE set by ccache-remote is settled), and BEFORE
`run_kernel_make_internal` builds its arrays (so `${CCACHE}`
substitution sees the real value). Ordering is fixed by core, not
by alphabet — order-independent by construction.
Changes:
* core: `prepare_compilation_vars` now calls a new extension hook
`compile_prepare_vars`. Default behavior with no extensions is
unchanged (passthrough).
* extension: `compile_prepare_vars__ccache` exports CCACHE / PATH
(prepended with /usr/lib/ccache) / CCACHE_DIR (when PRIVATE_CCACHE)
/ CCACHE_UMASK (shared-cache default). The export is enough for
`${CCACHE}` substitution, kernel-make.sh's existing CCACHE_DIR
pass-through, and uboot.sh's PATH/CCACHE_DIR pass-through. The
`*_make_config__ccache` hooks now only inject CCACHE_UMASK into
the env-i make envs (which core does not auto-pass, unlike
CCACHE_DIR).
* extension_prepare_config__ccache shrinks back to the mutex check
against competing compile-cache extensions.
* shim in main-config.sh honors the `EXT` alias (Armbian falls back
to ${EXT} when ENABLE_EXTENSIONS is empty — extensions.sh:144),
so `EXT=ccache-remote` triggers auto-enable just like
`ENABLE_EXTENSIONS=ccache-remote`.
Addresses codex feedback on commit 7 (head 05bf8ee): 1 P1
'set ccache env before make arguments are built' + 1 P2
'honor EXT when detecting ccache-remote'.
The third finding (P2 'preserve late USE_CCACHE overrides' from
userpatches/lib.config or user_config hook) is solved automatically
by this move: `compile_prepare_vars` runs late enough to see those
overrides; the extension still needs to be enabled by the shim, but
even when it is and USE_CCACHE arrives late, the env-setup hook
fires after the late assignment and respects it.
Assisted-by: Claude:claude-opus-4.7
|
@codex review Commit 8 (head Now:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2185ea647c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ "${USE_CCACHE}" == "yes" \ | ||
| || "${PRIVATE_CCACHE}" == "yes" \ | ||
| || "${_ext_list}" == *,ccache-remote,* ]]; then |
There was a problem hiding this comment.
When users keep the legacy USE_CCACHE=yes or PRIVATE_CCACHE=yes in userpatches/lib.config, this condition has already run; lib.config is sourced later in do_main_configuration, before extension_prepare_config, so the variables become yes but the ccache extension was never enabled. Previously prepare_compilation_vars checked these toggles after lib.config was sourced, so this supported legacy path now silently loses the CCACHE/PATH setup and builds without ccache. Move this auto-enable decision after the late user overrides, before initialize_extension_manager.
Useful? React with 👍 / 👎.
… extension init Two P2 findings from codex round 4 (head 2185ea6). * Clear stale CCACHE in prepare_compilation_vars (P2 'clear stale CCACHE when no backend is enabled'): The previous in-core ccache block had an explicit `else CCACHE=""` branch. Commit 6 dropped it; commit 8 reintroduced the call but without resetting CCACHE first, so in persistent shells (multi-build flows where an earlier USE_CCACHE=yes build left CCACHE=ccache exported) a subsequent USE_CCACHE=no build would still pick up the stale wrapper through ${CCACHE} substitution and surprise the user. Reset CCACHE to empty before the compile_prepare_vars hook fires; extensions explicitly assign it when they want the wrapper engaged. * Move the auto-enable shim closer to initialize_extension_manager (P2 'enable ccache after late lib.config overrides'): The shim was at the top of do_main_configuration, well before late user overrides (CLI defaults applied later in the function, EXT/ ENABLE_EXTENSIONS values mutated by other phases). Move it to fire immediately before initialize_extension_manager so all in-scope sources of USE_CCACHE / PRIVATE_CCACHE / ENABLE_EXTENSIONS are visible. lib.config is sourced after initialize_extension_manager (line 443-448 explicit comment 'it is too late to define hook functions or add extensions in lib.config') — that case remains a framework-level limitation outside our scope; emit a clear warning in prepare_compilation_vars so users in that scenario are nudged to migrate to ENABLE_EXTENSIONS=ccache. Addresses codex feedback on commit 8 (head 2185ea6): 2 P2. Assisted-by: Claude:claude-opus-4.7
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4a7cdb673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [[ ( "${USE_CCACHE}" == "yes" || "${PRIVATE_CCACHE}" == "yes" ) && -z "${CCACHE}" ]]; then | ||
| display_alert "USE_CCACHE / PRIVATE_CCACHE set, but no compile-cache extension is active" \ | ||
| "likely set in userpatches/lib.config — move to ENABLE_EXTENSIONS=ccache to engage the wrapper" "warn" |
There was a problem hiding this comment.
When USE_CCACHE=yes or PRIVATE_CCACHE=yes is set from userpatches/lib.config or a later user_config hook, this branch now only emits a warning because the ccache extension was not enabled before initialize_extension_manager. Those late overrides used to work: the old prepare_compilation_vars checked the toggles at this same phase and exported CCACHE/updated PATH, so existing user configurations that enable ccache there silently lose compiler caching after this change unless they migrate to ENABLE_EXTENSIONS=ccache.
Useful? React with 👍 / 👎.
| local _ext_list=",${ENABLE_EXTENSIONS:-${EXT:-}}," | ||
| if [[ "${USE_CCACHE}" == "yes" \ | ||
| || "${PRIVATE_CCACHE}" == "yes" \ | ||
| || "${_ext_list}" == *,ccache-remote,* ]]; then |
There was a problem hiding this comment.
When users request multiple extensions with the space-separated form accepted by initialize_extension_manager (for example ENABLE_EXTENSIONS="foo ccache-remote" or the same via EXT), _ext_list becomes ,foo ccache-remote, and this pattern does not match *,ccache-remote,*. In that scenario the shim never enables ccache, while ccache-remote still sets USE_CCACHE/PRIVATE_CCACHE later in its prepare hook, so remote ccache builds lose the local ccache wrapper and fall into the warning path instead of working as before.
Useful? React with 👍 / 👎.
| for other in "${__ext_ccache_conflicting_exts[@]}"; do | ||
| if [[ ",${ENABLE_EXTENSIONS:-}," == *",${other},"* ]]; then | ||
| exit_with_error "${EXTENSION}: 'ccache' and '${other}' extensions are mutually exclusive — choose one compile-cache backend" |
There was a problem hiding this comment.
If users enable extensions through the documented EXT alias, e.g. EXT=ccache,sccache with a user-provided sccache extension, this mutex check never sees the competing backend because it only inspects ENABLE_EXTENSIONS. The extension manager later loads both entries from EXT, so both cache backends can install their compiler wrappers and pre/post hooks despite the intended mutual exclusion.
Useful? React with 👍 / 👎.
… space-separated lists Three P2 findings from codex round 5 (head d4a7cdb). * Preserve legacy USE_CCACHE-in-lib.config flow (P2 'preserve late USE_CCACHE overrides'): Before this refactor, prepare_compilation_vars did the env wiring directly at compile phase — so users keeping USE_CCACHE=yes in userpatches/lib.config got working ccache without enabling any extension. Commit 9 only emitted a warning in that scenario, which silently lost caching for those configs. Restore the env setup inline (CCACHE, PATH prefix, CCACHE_DIR for PRIVATE_CCACHE, CCACHE_UMASK for shared cache) when the toggle is set but no extension hook engaged ${CCACHE}. The warning still fires so users know to migrate to ENABLE_EXTENSIONS=ccache for the richer flow, but their builds keep working. * Honor EXT alias + normalize delimiters in mutex / auto-enable checks (P2 'normalize space-separated extension lists' + P2 'honor EXT when checking cache-backend conflicts'): initialize_extension_manager accepts both comma- and space-separated lists, and falls back to ${EXT} when ENABLE_EXTENSIONS is empty (extensions.sh:144). The auto-enable shim and the per-extension mutex check now resolve the active list the same way: take ${ENABLE_EXTENSIONS:-${EXT:-}}, replace whitespace runs with commas, then do the substring check. Without this, EXT=ccache,sccache or ENABLE_EXTENSIONS="foo ccache-remote" bypassed both auto-enable and the cache-backend mutex. Assisted-by: Claude:claude-opus-4.7
|
Codex Review: Didn't find any major issues. Can't wait for the next one! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Phase '-1' of the sccache-readiness refactor: extract Armbian's
ccache compile-cache backend into a swappable extension, while keeping
the existing
USE_CCACHE=yes/PRIVATE_CCACHE=yesuser togglesworking with zero migration churn. After this PR a parallel cache
backend (sccache, distcc-style helper, …) can land as a sibling
extension without touching core.
Codex-trap mirror of an upstream PR that has not been opened yet;
the intent is to vet the design here (with codex review) before
approaching maintainers in
armbian/buildwith the same series.Commits
chore(crust): drop dead binutils_flags_crust=""— pre-existingSC2034 since 2023 (
8278dc5e4); copy-pasted skeleton fromatf.shwithout the gcc/ld probe block that would have populatedit.
fix(atf): use ${CCACHE} variable instead of hardcoded ccache—latent bug:
atf.sh:86hardcoded'ccache ${ATF_COMPILER}',ignoring
USE_CCACHE/PRIVATE_CCACHE. Other compile paths(kernel-make.sh:60, crust.sh:55,59) already used
${CCACHE}.feat(compilation): add atf_make_config and crust_make_config extension hooks— symmetric to the existingkernel_make_configand
uboot_make_config. Extensions can inject env vars / makeparams right before the make invocation.
feat(compilation): add do_with_compile_wrapper and compile_wrapper_pre/post hooks— backend-agnostic wrapper withpre/post extension hooks. Cleanup-handler is registered before
the pre hook runs (addresses codex P2 finding on the earlier
bridge revision: a long-lived helper started by one pre hook is
cleaned up even if a later pre hook fails or the build is
interrupted).
feat(extensions): add extensions/ccache.sh implementing compile_wrapper + make_config hooks— ccache logic moves out ofcore into an opt-in extension that implements
extension_prepare_config__ccache, the four*_make_config__ccachehooks (kernel / u-boot / atf / crust), and the
compile_wrapper_pre/post__ccachehooks. Mutex check againstcompeting compile-cache extensions is enforced in
extension_prepare_config__ccache. Also adds an auto-enable shimin
main-config.sh: whenUSE_CCACHE=yes/PRIVATE_CCACHE=yesis set and no other cache-backend extension is requested, the
ccacheextension is appended toENABLE_EXTENSIONS.refactor(compilation): migrate kernel/uboot call sites to do_with_compile_wrapper— switch the three existing call sites(
kernel.sh:259,263anduboot.sh:273) to the generic wrapper.refactor(compilation): remove legacy ccache.sh and inline ccache setup— deletelib/functions/compilation/ccache.shentirelyand the now-redundant ccache block in
prepare_compilation_vars.call-stack-analyze.py's skip-list is updated to the new wrappername. The legacy
ccache_post_compilationextension hook ispreserved (invoked from inside
compile_wrapper_post__ccache) for backward compatibility with3rd-party extensions that listened to it.
Behavior change for users
None.
USE_CCACHE=yes/PRIVATE_CCACHE=yescontinue to enableccache exactly as before, just routed through the extension.
Net diff
ccache block removed, two small hook additions in atf/crust).
extensions/ccache.sh).What this enables
extensions/sccache.shas a sibling drop-in replacement (Phase 0follow-up): mirror structure, different binary, optional cloud
backends (file/redis/s3/webdav/gha) — all extension-internal.
Core unchanged.
extensions/sccache-dist-worker.shfor distributed compilationwith sccache-dist scheduler/workers — provisioned via cloud-init,
activates only when the extension is enabled. Core unchanged.
RUSTC_WRAPPER— small follow-up in thekernel-rust extension. Core unchanged.
Earlier codex findings on this branch
The first revision of commit 4 used an in-core bridge
(
compile_wrapper_pre__ccache_core/_post__ccache_coredefinedinside
lib/functions/compilation/ccache.sh). Codex P2 flagged thatthe extension manager scans all
*__*functions, so the bridgewould fire unconditionally on every
do_with_compile_wrappercall —including for sccache or no-extension passthrough flows. Resolved
by removing the bridge entirely and letting
extensions/ccache.shown the hook implementations (they fire only when the extension is
enabled). Second P2 (cleanup-handler timing) addressed in commit 4
by registering the cleanup handler before the pre hook runs.
Asking codex for fresh review
@chatgpt-codex-connector review
Please re-verify on the 7-commit chain ending at
29a4bb93b:add_cleanup_handler …firesbefore any pre hook. Does that guarantee the post hook runs
exactly once across success / failure / SIGINT paths?
enable_extension "ccache"runsin
main_config_redefine_user_targetright afterUSE_CCACHEisdeclared. Late enough that user configs have set the toggle,
early enough that the extension manager picks it up. Any concern?
__ext_ccache_conflicting_exts=("sccache")list lives in the extension; when sccache lands later, both
extensions will need to know about each other. Is a centralized
"compile-cache provider" marker variable cleaner, or is the
per-extension list acceptable?
ccache_post_compilationhook — preserved insidecompile_wrapper_post__ccachefor backward compatibility with3rd-party extensions. Should it also be invoked from the generic
do_with_compile_wrapperso non-ccache backends can opt into it,or is the current ccache-only routing correct?