build: fix mac CI ccache, only wrap darwin CC/CXX in env -u when include paths are set - #7540
Conversation
The unconditional 'env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH' prefix on darwin_CC/darwin_CXX silently disables ccache for every macOS cross-compile in CI: configure prepends ccache to CC, producing 'ccache env -u VAR /path/to/clang ...'. ccache treats 'env' as the compiler (type 'other'), consumes '-u VAR' as an option taking an argument, and then classifies the absolute clang path -- an existing file not starting with '-' -- as a second source file. Every compile is rejected as 'multiple_source_files' and falls back to the real compiler, so the mac CI job recompiles all ~1000 objects from scratch (~22 min) on every run while saving a 17 KB ccache cache entry. The prefix exists to stop Guix (contrib/guix/libexec/build.sh) from leaking native-GCC include paths into the darwin cross-build. CI never sets those variables, so emit the prefix only when C_INCLUDE_PATH or CPLUS_INCLUDE_PATH is actually defined: Guix behavior is unchanged (variables set, and Guix does not use ccache), and CI gets a working mac ccache. depends build ids are unaffected in CI because gen_id hashes compiler -v output, which is byte-identical with and without the prefix when the variables are unset.
|
🕓 Ready for review — 16 ahead in queue (commit 1dc8e9b) |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe Darwin host configuration adds a conditional Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
I don't fully follow this one; but ccache for Mac builds have been broken since Jan, we should probably fix that :) this is fable's proposal. |
|
Much better |
There was a problem hiding this comment.
Final validation — Codex + Sonnet
The change correctly applies the Darwin compiler environment wrapper only when either include-path variable is defined, preserving Guix header isolation while allowing ccache to invoke clang directly in CI. The implementation is narrowly scoped, handles unset, set, and set-but-empty variables correctly, and introduces no actionable defects.
Source: reviewers gpt-5.6-sol (general, dash-core-commit-history) and claude-sonnet-5 (general, dash-core-commit-history); final verifier gpt-5.6-sol (fallback after two unparseable claude-sonnet-5 verifier attempts).
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— dash-core-commit-history (completed) - Verifier:
gpt-5.6-sol— final-verifier (fallback) - Sonnet reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— dash-core-commit-history (completed),claude-sonnet-5— general (completed)
| # TODO: remove C_INCLUDE_PATH when it is indeed useless | ||
| # https://github.com/bitcoin/bitcoin/pull/30451 has been partiall reverted in #7184 and should be re-applied | ||
| darwin_CC=env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH $(clang_prog) --target=$(host) \ | ||
| # C_INCLUDE_PATH/CPLUS_INCLUDE_PATH leak native-toolchain headers into the |
There was a problem hiding this comment.
keep TODO; bitcoin#30451 has been done partially and this workaround should be removed when 30451 is fully backported.
![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: 1dc8e9b396
ℹ️ 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".
5950828 fix(ci): hash all depends recipe inputs (pasta) Pull request description: ## Issue being fixed or feature implemented The depends cache key did not cover every file used to derive depends package build IDs. In [run 30864594362](https://github.com/dashpay/dash/actions/runs/30864594362/job/91859430920?pr=7540), PR #7540 changed `depends/hosts/darwin.mk`, but cache lookup still reported a hit and skipped the dedicated depends build. The macOS source job then rebuilt packages for about 17 minutes. ## What was done? - Include the complete static depends recipe inputs in the cache key: the Makefiles, ID generator, config files, builders, hosts, packages, and patches. - Check out the full tracked `depends/` directory in the cache-check job so those inputs are available to `hashFiles`. This causes a true cache miss when a relevant depends input changes, allowing the existing depends job/artifact handoff to provide matching packages to source jobs. ## How Has This Been Tested? - Ran `git diff --check`. - Ran `actionlint .github/workflows/build-depends.yml`. It reports existing diagnostics for the repository's custom checkout input and pre-existing shell-style issues; no diagnostics are caused by this change. ## Breaking Changes None. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone This pull request was created by Codex. Top commit has no ACKs. Tree-SHA512: 893f4ddf6d0d2419384c70bfd95e1585baae3401cdba30def8f0433f952fe9c1e59bcad351b7a20c6d28b69f525c94c13a8e8fdb2e93aa12af4ec28e217171cc
3 participants
Issue being fixed or feature implemented
ccache is silently disabled for the entire
mac-buildCI job, and has been since d201e43 (2026-01-19) re-added theenv -uprefix todarwin_CC/darwin_CXX(it was previously broken from 2020-08 until the bitcoin#30451 backport in 1d8868b removed the prefix in 2025-02).Every mac CI run shows:
and recompiles all objects from scratch (~22 min compile in run 30848679596), while saving a 17 KB ccache cache entry.
Root cause: configure prepends ccache to
CC, producingccache env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH /usr/bin/clang .... ccache treatsenvas the compiler (typeother), consumes-u VARas an option taking an argument, then classifies the absolute clang path — an existing file not starting with-— as a second source file.CCACHE_DEBUGshows the verdict:Every call is rejected and silently falls back to the real compiler, so the build succeeds and nothing ever flagged it. Current ccache master has the same parsing behavior; this is not fixed by upgrading ccache.
What was done?
The
env -uprefix exists to stop Guix (contrib/guix/libexec/build.shexportsC_INCLUDE_PATH="${NATIVE_GCC}/include"etc.) from leaking native-toolchain headers into the darwin cross-build — the conflict d201e43 fixed. CI never sets those variables, so the prefix does nothing there except break ccache.The prefix is now emitted only when
C_INCLUDE_PATHorCPLUS_INCLUDE_PATHis actually defined (checked via$(origin ...), so set-but-empty still counts as set):$(clang_prog)→ ccache wraps clang directly and caching works.Upstream removed the prefix entirely (bitcoin#30451) because their Guix build.sh no longer exports these variables; ours still does, so the conditional keeps the Guix protection while re-applying the effect of bitcoin#30451 everywhere else. The stale TODO comments pointing at bitcoin#30451/#7184 are replaced with an explanation of both constraints.
Cached depends are not invalidated by this change:
depends/gen_idhashes compiler-voutput, which is byte-identical with and without the prefix when the variables are unset (verified below).How Has This Been Tested?
All in
ubuntu:24.04(same ccache 4.9.1 + clang 18 as the CI image), plus a make-level check:ccache env -u C_INCLUDE_PATH -u CPLUS_INCLUDE_PATH /usr/bin/clang --target=x86_64-apple-darwin -isysroot... -nostdlibinc ... -c t.c→Uncacheable calls: 1/1,CCACHE_DEBUGlog showsResult: multiple_source_files. Bareclanginstead of an absolute path does not trigger it, confirming the source-file misparse.Cacheable calls: 2/2, second compile is a cache hit.depends/hosts/darwin.mk: with both variables unsetdarwin_CChas no prefix; withC_INCLUDE_PATHset (or set-but-empty) the prefix is emitted.{clang -v; clang -v -E -xc -o /dev/null -}output (thegen_idpreimage components) is byte-identical with and without theenv -uprefix when the variables are unset, so CI's cached depends remain valid.Guix builds are unaffected by construction (the conditional evaluates true in that environment), but a
guix-buildrun on this PR will confirm.Breaking Changes
None.
Checklist: