Repository navigation
Keep outdated :no_linkage build dependencies - #24290
MikeMcQuaid merged 3 commits into
Conversation
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Potentially happy to merge this but more questions again. Would be good to add dedicated tests for this and the last PR to verify functionality doesn't change when building bottles.
Happy to hear! Yes, tests are good! You argued here (#15927) that testing was not feasible, would you say it's more possible to add here, and/or for the code in my previous one: #24273. |
Sorry, I can't see where I said that, can you quote me?
add here but include for the previous one. I'm thinking of more in |
Here: #15927 (comment)
Makes sense, I'll take a look (likely tomorrow because I need some family time 😃) |
Ah. You can ignore anything I said about testing pre-AI 😉 |
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly and concisely describes the main change: keeping outdated build dependencies marked :no_linkage. |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
|
@MikeMcQuaid Tests added! |
- A build dependency that is only run, like `llvm` used as a compiler, isn't built against, so its `compatibility_version` doesn't matter. Requiring a match makes e.g. `ccache` on Xcode 14 rebuild `llvm` 23 even though the installed `llvm` 22 can compile it. - Let formulae tag such build dependencies `:no_linkage`, which already means the formula doesn't link against them, and keep them when outdated. The dependencies they were built against are still checked.
- Check that `--build-bottle`, with or without `--build-from-source`, still upgrades outdated build dependencies under both keep rules: a matching `compatibility_version` (Homebrew#24273) and `:no_linkage`. - Also check that a build dependency's own dependencies are upgraded when building a bottle, and that `:no_linkage` alone doesn't keep an outdated runtime dependency.
- Check that the old behaviour stays for an outdated build dependency with a different `compatibility_version`, an outdated runtime dependency with a matching one and a keg whose install receipt lists no runtime dependencies. - Check that a `:no_linkage` build dependency is still upgraded if it isn't opt-linked or is missing required options.
2513d69 to
ecc49c6
Compare
- ccache 4.14 uses C++20 `std::ranges` algorithms that Apple clang 14 (Xcode 14.2 and older) doesn't provide, so build with `llvm` there, like `mysql@8.4` and others do. Declaring `llvm` also puts `llvm-ar` on the build `PATH`, which CMake needs for `ENABLE_IPO`. - ccache only uses llvm's clang as a compiler and doesn't link against it, so mark it `:no_linkage`. Since Homebrew/brew#24290, an installed but outdated `llvm` is then kept, not rebuilt.
- ccache 4.14 uses C++20 `std::ranges` algorithms that Apple clang 14 (Xcode 14.2 and older) doesn't provide, so build with `llvm` there, like `mysql@8.4` and others do. Declaring `llvm` also puts `llvm-ar` on the build `PATH`, which CMake needs for `ENABLE_IPO`. - ccache only uses llvm's clang as a compiler and doesn't link against it, so mark it `:no_linkage`. Since Homebrew/brew#24290, an installed but outdated `llvm` is then kept, not rebuilt. - Hide `llvm` from the `PATH` again for the tests. Its `llvm-profdata` can't read the profiles Apple clang 14 writes, so `test.profiling` and `test.profiling_clang` fail. Without it they're skipped, as on newer Xcode.
2 participants
Follow-up to #24273, which lets source builds keep an outdated build dependency when its installed
compatibility_versionmatches the current formula.That works for most build dependencies, but not for one that a formula only runs while its
compatibility_versionis bumped for linking reasons. The common case isllvmused as a compiler:llvmbumpscompatibility_versionwith each major version because libLLVM's ABI changes. That matters for formulae that link libLLVM, likerust, but not for one that only compiles withclang.This lets formulae tag such build dependencies
:no_linkage, which already declares that the formula doesn't link against the dependency, and keeps them when outdated regardless ofcompatibility_version. Everything else from that change still applies: nothing is kept with--build-bottle, and a kept build dependency is still upgraded if this install would incompatibly upgrade something it was built against.How I got here
On my MacBook Air (macOS 12.7.6, Xcode 14.2, so no bottles),
brew install ccachestarted by rebuildingllvmand spent three hours on it before I cancelled. Keeping compatible build dependencies avoided that rebuild forruby'srustbuild dependency, but ccache itself needsllvmto build on Xcode 14 (Homebrew/homebrew-core#319277). Withllvmdeclared as a build dependency, brew rebuildsllvm23 first: myllvm22.1.8 hascompatibility_version1 and the formula has 2, even thoughllvm22'sclangcompiles ccache fine. With this change and that ccache PR, which declaresdepends_on "llvm" => [:build, :no_linkage], brew keptllvm22.1.8 and built ccache 4.14.1_2 from source in about six minutes.brew test ccacheandbrew audit --strict ccachepass.Changes
FormulaInstaller: when building from source, keep an outdated:no_linkagebuild dependency if it's installed and opt-linked, whatever itscompatibility_version. Other build dependencies still need a matchingcompatibility_version.:no_linkageDSL docs and the Formula Cookbook.:no_linkagebuild dependency with a differentcompatibility_version.brew benchmarkresults.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?Claude Code (Claude Opus 5.5) helped trace the dependency resolution code and write the change, docs and spec. So far it's verified with
brew styleon the changed files and by installing ccache on my macOS 12 machine with this branch and the linked ccache PR, which keptllvm22 instead of rebuilding it.brew typecheckand theformula_installerspecs haven't run locally yet.Summary by CodeRabbit
:no_linkagebuild dependencies without unnecessarily requiring a compatible installed version. Dependencies may be skipped when they are not linked or have missing options; other build dependencies continue to follow compatibility-version checks.:no_linkagebuild dependencies can use an outdated installed version during source builds.