Embed executables in API JSON#22301
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the dedicated internal/executables.txt download path and instead embeds per-formula executable lists in the formula/internal API JSON. The client-side cache file is regenerated from that JSON whenever update-report writes name/alias lookups, and brew which-formula/brew exec rely on brew update --auto-update to refresh it rather than performing their own curl.
Changes:
- Add
executablestoFormulaStructand write it into per-formula JSON and internalpackages.<tag>.jsonfromExecutablesDBdata committed in the tap. - Add
Homebrew::API.write_executables_file!and call it fromAPI::Formula.write_names_and_aliases/API::Internal.write_formula_names_and_aliasesto regenerateinternal/executables.txtfrom JSON. - Drop the dedicated curl +
homebrew.commandnotfoundfetch path fromupdate.sh/which-formula.sh(nowensure_executables_filejust relies on auto-update).
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Library/Homebrew/api.rb | New write_executables_file! writing internal/executables.txt from formula JSON. |
| Library/Homebrew/api/formula.rb | Call write_executables_file! after writing formula names/aliases. |
| Library/Homebrew/api/internal.rb | Same call from the internal-API path. |
| Library/Homebrew/api/formula_struct.rb | Add executables field to FormulaStruct. |
| Library/Homebrew/dev-cmd/generate-formula-api.rb | Load ExecutablesDB and inject executables into generated formula hashes. |
| Library/Homebrew/dev-cmd/generate-internal-api.rb | Same injection for internal packages JSON. |
| Library/Homebrew/executables_db.rb | New to_hash accessor for read-only consumers. |
| Library/Homebrew/cmd/update.sh | Remove conditional fetch of internal/executables.txt. |
| Library/Homebrew/cmd/which-formula.sh | Replace download_and_cache_executables_file with ensure_executables_file that triggers brew update --auto-update. |
| Library/Homebrew/cmd/exec.sh | Update caller to ensure_executables_file. |
| Library/Homebrew/test/api/* | Add coverage for executables field and write_executables_file!. |
| Library/Homebrew/test/dev-cmd/generate-*-api_spec.rb | Verify generator embeds executables read from ExecutablesDB. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ee2dc80 to
9d8a8d1
Compare
Rylan12
left a comment
There was a problem hiding this comment.
Nice, I think this makes a lot of sense!
My main concern would be that we're introducing a circular dependency where generating the API depends on the API itself. I'm pretty sure we're not actually doing that here because the /api/internal/executables.txt file is actually sourced from GitHub packages, not any in-api state.
One question, but otherwise good to go assuming my analysis above is correct!
- Avoid fetching `executables.txt` as a separate API artefact. - Populate generated JSON from GitHub Packages metadata. - Keep `which-formula` data generated from active formula JSON. - Fall back to GitHub Packages data during the JSON transition. - Drop obsolete `--skip-update` command options. - Keep stale `HOMEBREW_BOOTSNAP_GEM_PATH` from breaking coverage runs. - Use the correct Linux mtime check for the executables database.
9d8a8d1 to
2a13176
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 30 out of 32 changed files in this pull request and generated 1 comment.
Files not reviewed (2)
- Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/exec.rbi: Language not supported
- Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/which_formula.rbi: Language not supported
executables.txtas a separate API artefact.which-formuladata generated from active formula JSON.brew lgtm(style, typechecking and tests) with your changes locally?OpenAI Codex 5.5 xhigh with local review, tweaks and testing.