Add which entry command by ooye-sanket · Pull Request #21790 · Homebrew/brew

ooye-sanket · 2026-03-21T07:47:53Z

Adds brew which-entry <formula> a developer command that generates
a single executables.txt entry for a formula by reading
sh.brew.path_exec_files from its bottle manifest.

This is intended to replace the batch brew which-update job with a
per-formula update triggered by homebrew/core CI on each merge.
What it does

Fetches the bottle manifest and reads sh.brew.path_exec_files
With --output-db=<file>: upserts the formula's line in the DB file
and removes it if the formula is disabled, deprecated, or deleted
Without --output-db=: prints the DB line to stdout

Relationship to other PRs
The companion homebrew/core PR (Homebrew/homebrew-core#272976) slims
the existing workflow down to just call brew which-entry per formula
detected by brew test-bot --only-formulae-detect, replacing the
current daily batch job entirely.

@MikeMcQuaid please have a look!

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests for your changes? Here's an example.
Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.

MikeMcQuaid

Nice work @ooye-sanket! Looks good so far. Please add a very basic integration test and args check for which-entry. Also, please inline functions that are only used once as there's a fair bit of indirection right now.

MikeMcQuaid · 2026-03-21T08:31:53Z

+      rescue JSON::ParserError
+        []


Probably worth a opoo warning of the exception in this case?

ooye-sanket · 2026-03-21T09:34:36Z

Thanks @MikeMcQuaid! I've pushed the changes:

Inlined cached_manifest, fetch_manifest, read_db, and remove_entry .
Added opoo warning when JSON::ParserError is raised so failures aren't silent
Added the basic spec with args check

Let me know if the level of inlining looks right or if you'd like it structured differently!

MikeMcQuaid

Thanks! Please ensure CI syntax job passing before asking for re-review.

MikeMcQuaid · 2026-03-22T14:11:29Z

+        if db_path
+          existing = db_path.exist? ? db_path.readlines(chomp: true).reject(&:empty?) : []
+          lines = existing.reject { |l| l.start_with?("#{formula.full_name}(") }
+          lines << line if line


Maybe use filter_map?

github-actions · 2026-04-13T00:32:02Z

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

MikeMcQuaid

Thanks, looking good, a few small things!

MikeMcQuaid · 2026-04-14T13:47:41Z

+
+      sig { override.void }
+      def run
+        db_path = Pathname(T.unsafe(args).output_db) if T.unsafe(args).output_db


This shouldn't be necessary, you may need to run brew typecheck --update to avoid the need for T.unsafe.

@ooye-sanket Can you take another look at this?

MikeMcQuaid · 2026-04-14T13:48:38Z

+      sig { params(db_path: Pathname, name: String, line: T.nilable(String)).void }
+      def write_db(db_path, name, line)
+        lines = db_path.readlines(chomp: true).compact_blank if db_path.exist?
+        lines = (lines || []).reject { |l| l.start_with?("#{name}(") }


Suggested change

lines = (lines || []).reject { |l| l.start_with?("#{name}(") }

lines = Array(lines).reject { |l| l.start_with?("#{name}(") }

This is not resolved

Also, there's an inconsistency here, right? We reject lines that start with name( but we don't actually write the version in parentheses anymore.

Fixed as now uses Array(lines) and the reject pattern is corrected to "#{name}:" to match the new format without pkg_version.

You need to support both the old and new formats

Rylan12

Sorry for the review delay here. I agree with Mike's feedback, and also have one other thought.

More generally, I'm not sure whether this actually needs to be a brew command at all. I will leave a comment on the other PR with more details

Edit: oh, one more thing, we should remove the existing command-not-found-db-update.yml workflow

ooye-sanket · 2026-04-17T06:59:14Z

hey @Rylan12 The reason for keeping it as a brew command is that it already handles formula loading, bottle manifest caching, disabled/deprecated checks, and DB file management in a tested, typed Ruby context. Doing all that reliably in bash would be fragile. But happy to discuss if there's a simpler scope you have in mind.

MikeMcQuaid · 2026-04-17T08:02:02Z

Let's keep it as Ruby for now and we can see how it's looking when the other PR is ready.

Rylan12 · 2026-04-18T01:40:35Z

I don't mind keeping it in Ruby for now, so this is not a blocker.

it already handles formula loading, bottle manifest caching, disabled/deprecated checks, and DB file management in a tested, typed Ruby context. Doing all that reliably in bash would be fragile

I think doing it in bash would actually let us bypass these complexities altogether, resulting in a simpler system overall. There would be no need to load formulae at all, and the deprecated/disabled info is available via the same bottle manifest that we already need to use.

Fair enough on the DB management, although I think a reasonably well-crafted sed line would do the trick.

I haven't attempted to write it out, so I could be wrong

ooye-sanket · 2026-04-22T15:49:14Z

Hey @MikeMcQuaid I've made all the changes (dropped pkg_version, fixed the reject pattern, removed T.unsafe) but I'm unable to run brew typecheck --update locally to regenerate the RBI file. Could you help run it or point me to how to get past this? Everything else should be good to go!

MikeMcQuaid · 2026-04-22T15:52:18Z

@ooye-sanket what happens when you run it locally?

MikeMcQuaid · 2026-04-23T16:04:06Z

@ooye-sanket nudge on above question!

MikeMcQuaid

Waiting on author answering question.

ooye-sanket · 2026-04-27T09:47:26Z

Hey @MikeMcQuaid, sorry for the delay! I ran brew typecheck --update locally and it worked successfully it generated the new sorbet/rbi/dsl/homebrew/dev_cmd/which_entry.rbi RBI file which I've now committed. I've also rebased onto latest upstream/main. Ready for re-review

Note: the brew typecheck CI failure is in extend/os/mac/keg_relocate.rb and extend/os/mac/keg.rb these are pre-existing errors unrelated to this PR.

CC @Rylan12

MikeMcQuaid · 2026-04-27T13:22:49Z

Note: the brew typecheck CI failure is in extend/os/mac/keg_relocate.rb and extend/os/mac/keg.rb these are pre-existing errors unrelated to this PR.

That's not true: these are passing on main: https://github.com/Homebrew/brew/actions/runs/24983715091/job/73151721181

MikeMcQuaid

Still waiting on author

ooye-sanket · 2026-04-30T07:52:42Z

Hey @MikeMcQuaid, CI is now green! , The earlier typecheck failure was caused by our update accidentally removing mach_o_shim.rbi I've now restored it. Ready for re-review! 🙏

MikeMcQuaid · 2026-04-30T10:40:35Z

+
+      sig { override.void }
+      def run
+        db_path = Pathname(T.unsafe(args).output_db) if T.unsafe(args).output_db


@ooye-sanket Can you take another look at this?

ooye-sanket · 2026-05-01T12:28:43Z

Hey @MikeMcQuaid, sorry for the noise! All 3 issues fixed.
CI is green and ready for re-review!

Rylan12 · 2026-05-02T02:03:15Z

+
+      sig { override.void }
+      def run
+        db_path = Pathname(T.must(args.output_db)) if args.output_db


IMO we should just make this argument required and raise a usage error if it's not passed. Or accept it as a named arg

Fixed --output-db is now required and raises a UsageError if not passed.

Rylan12 · 2026-05-02T02:08:08Z

+      sig { params(db_path: Pathname, name: String, line: T.nilable(String)).void }
+      def write_db(db_path, name, line)
+        lines = db_path.readlines(chomp: true).compact_blank if db_path.exist?
+        lines = (lines || []).reject { |l| l.start_with?("#{name}(") }


This is not resolved

Rylan12 · 2026-05-02T02:10:44Z

+      sig { params(db_path: Pathname, name: String, line: T.nilable(String)).void }
+      def write_db(db_path, name, line)
+        lines = db_path.readlines(chomp: true).compact_blank if db_path.exist?
+        lines = (lines || []).reject { |l| l.start_with?("#{name}(") }


Also, there's an inconsistency here, right? We reject lines that start with name( but we don't actually write the version in parentheses anymore.

Rylan12 · 2026-05-02T02:22:45Z

@ooye-sanket, some of the issues Mike pointed out don't actually seem to have been fixed yet. When you fix those, can you please run the commands locally on your machine and comment the output here to help verify that this works as expected?

It would be helpful to see a sample executables.txt with pkg_versions and and outdated exe list, then show the brew which-entry command and it's output, and then the fixed executables.txt file

MikeMcQuaid

Comments not addressed. Do not mark as resolved or request re review until 1) all comments past and present resolved and 2) all @Rylan12's comments and questions answered.

If I'm requested for review again and this still isn't ready: I'm just likely to close this out, sorry. Please raise the bar for yourself here. Thank you ❤️

…fix write_db, remove old workflow

ooye-sanket · 2026-05-02T08:56:03Z

Hey @MikeMcQuaid and @Rylan12, all comments are now addressed in the latest commits.

--output-db is now required raises UsageError if not passed
write_db now uses Array(lines) and correctly rejects name: instead of name(
executables_from_manifest now uses formula.bottle.github_packages_manifest_resource with downloaded? and cached_download no more filesystem globbing
Removed command-not-found-db-update.yml as requested

Here's proof the command works locally:

Entries are sorted, idempotent, and correctly formatted as name:executables.

NOTE : The only failing CI check is Docker / docker (x86_64 Ubuntu 22.04) which is a GitHub infrastructure issue apt-get update failing with permission errors, unrelated to our code. All 32 other checks are passing

Rylan12 · 2026-05-02T17:29:39Z

+      sig { params(db_path: Pathname, name: String, line: T.nilable(String)).void }
+      def write_db(db_path, name, line)
+        lines = db_path.readlines(chomp: true).compact_blank if db_path.exist?
+        lines = (lines || []).reject { |l| l.start_with?("#{name}(") }


You need to support both the old and new formats

ooye-sanket · 2026-05-03T06:51:19Z

Hey @Rylan12, all feedback addressed! please have a look

Rylan12

@ooye-sanket, this implementation does not work correctly for any formulae that have multiple executables.

For future pushes, please include proof that you've tested the changes and that they work correctly for the standard case and also edge cases. Since this will run on every formula update, it's imperative that the code is sound and can handle all of the possible situations that may arise.

To help with this, unit tests would be useful for all changes (aside from a full integration tests, which are okay to skip because they're too slow).

Rylan12 · 2026-05-04T04:50:41Z

+        manifest_path = manifest_resource.cached_download
+        return [] unless manifest_path.exist?


I think .fetch is probably good enough here. It's called by default with verify_download_integrity: true, so I think we'll get an error before this point if we weren't able to download the manifest. I'd say we can just get rid of these lines since I'm not convinced they add additional safety. It seems appropriate to me for this to fail if the formula is bottled but unable to download the bottle manifest. That does seem error-worthy

Suggested change

manifest_path = manifest_resource.cached_download

return [] unless manifest_path.exist?

Rylan12 · 2026-05-04T04:52:19Z

+      rescue JSON::ParserError => e
+        opoo "Failed to parse bottle manifest for #{formula.name}: #{e.message}"
+        []


Let's get rid of this. BottleResource#manifest_annotations raises a nice error if the JSON doesn't parse, and I think we want a hard error here. Also it's a bit weird to rescue a JSON parse error when we don't actually call JSON.parse directly in this method.

Suggested change

rescue JSON::ParserError => e

opoo "Failed to parse bottle manifest for #{formula.name}: #{e.message}"

[]

Rylan12 · 2026-05-04T05:00:26Z

+    def path_exec_files
+      exec_files = manifest_annotations["sh.brew.path_exec_files"]
+      return [] if exec_files.blank?
+
+      exec_files.split.map { |f| File.basename(f) }.sort
+    end


This does not work correctly for annotations that have multiple entries. The entries are comma-separated, so split doesn't work.

Additionally, the File.basename doesn't belong in this method. This method should return the data directly, and leave it up to the caller how to process it.

Suggested change

def path_exec_files

exec_files = manifest_annotations["sh.brew.path_exec_files"]

return [] if exec_files.blank?

exec_files.split.map { |f| File.basename(f) }.sort

end

def path_exec_files

manifest_annotations.fetch("sh.brew.path_exec_files", "").split(",")

end

MikeMcQuaid

@ooye-sanket please stop requesting my review here, thanks. I'll review when @Rylan12 is ✅.

ooye-sanket mentioned this pull request Mar 21, 2026

workflows: add per-formula command-not-found DB entry update Homebrew/homebrew-core#272976

Open

7 tasks

MikeMcQuaid reviewed Mar 21, 2026

View reviewed changes

ooye-sanket requested a review from MikeMcQuaid March 21, 2026 09:35

MikeMcQuaid reviewed Mar 22, 2026

View reviewed changes

github-actions Bot added stale No recent activity and removed stale No recent activity labels Apr 13, 2026

ooye-sanket requested a review from MikeMcQuaid April 13, 2026 08:45

ooye-sanket force-pushed the add-which-entry-command branch from 9e3e0a6 to 00d9e78 Compare April 13, 2026 08:49

MikeMcQuaid reviewed Apr 14, 2026

View reviewed changes

MikeMcQuaid requested a review from Rylan12 April 14, 2026 13:50

Rylan12 reviewed Apr 17, 2026

View reviewed changes

Comment thread Library/Homebrew/dev-cmd/which-entry.rb Outdated

ooye-sanket requested a review from MikeMcQuaid April 22, 2026 15:49

MikeMcQuaid reviewed Apr 27, 2026

View reviewed changes

ooye-sanket force-pushed the add-which-entry-command branch from 91f72e6 to 64df19c Compare April 27, 2026 09:42

ooye-sanket requested review from MikeMcQuaid and Rylan12 April 27, 2026 10:33

MikeMcQuaid reviewed Apr 28, 2026

View reviewed changes

MikeMcQuaid removed the request for review from Rylan12 April 28, 2026 08:13

ooye-sanket requested a review from MikeMcQuaid April 30, 2026 07:52

MikeMcQuaid requested a review from Rylan12 April 30, 2026 10:10

MikeMcQuaid reviewed Apr 30, 2026

View reviewed changes

ooye-sanket added 10 commits May 1, 2026 12:00

dev-cmd: add which-entry command for executables DB

e7ace72

dev-cmd: fix brew style offenses in which-entry

634688b

dev-cmd: inline helpers, add opoo warning, add basic spec

712c2e5

improve as address by maintainer

3164a71

dev-cmd: fix brew style offenses in which-entry

7423140

dev-cmd: fix Sorbet typecheck errors in which-entry

2410819

dev-cmd: regenerate RBI files for which-entry

29e8def

dev-cmd: restore mach_o_shim.rbi removed by typecheck update

79f9a39

dev-cmd: remove T.unsafe, drop pkg_version, restore unrelated rbi change

edfa134

dev-cmd: fix Sorbet type error for args.output_db

50f706c

ooye-sanket force-pushed the add-which-entry-command branch from 5680029 to 50f706c Compare May 1, 2026 12:01

ooye-sanket requested a review from MikeMcQuaid May 1, 2026 12:28

Rylan12 reviewed May 2, 2026

View reviewed changes

MikeMcQuaid requested changes May 2, 2026

View reviewed changes

ooye-sanket added 2 commits May 2, 2026 07:38

dev-cmd: use github_packages_manifest_resource, require --output-db, …

63beb65

…fix write_db, remove old workflow

dev-cmd: fix which-entry spec to use integration test helper

8dcb4fe

ooye-sanket requested review from MikeMcQuaid and Rylan12 May 2, 2026 08:56

Rylan12 reviewed May 2, 2026

View reviewed changes

dev-cmd: address all Rylan12 feedback on which-entry

021e49e

ooye-sanket requested a review from Rylan12 May 3, 2026 07:28

Rylan12 requested changes May 4, 2026

View reviewed changes

MikeMcQuaid requested changes May 4, 2026

View reviewed changes

	lines = (lines \|\| []).reject { \|l\| l.start_with?("#{name}(") }
	lines = Array(lines).reject { \|l\| l.start_with?("#{name}(") }

		manifest_path = manifest_resource.cached_download
		return [] unless manifest_path.exist?

	rescue JSON::ParserError => e
	opoo "Failed to parse bottle manifest for #{formula.name}: #{e.message}"
	[]

Uh oh!

Conversation

ooye-sanket commented Mar 21, 2026

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ooye-sanket commented Mar 21, 2026

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Apr 13, 2026

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ooye-sanket May 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Rylan12 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ooye-sanket commented Apr 17, 2026

Uh oh!

MikeMcQuaid commented Apr 17, 2026

Uh oh!

Rylan12 commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ooye-sanket commented Apr 22, 2026

Uh oh!

MikeMcQuaid commented Apr 22, 2026

Uh oh!

MikeMcQuaid commented Apr 23, 2026

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

ooye-sanket commented Apr 27, 2026

Uh oh!

MikeMcQuaid commented Apr 27, 2026

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

ooye-sanket commented Apr 30, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ooye-sanket commented May 1, 2026

Uh oh!

ooye-sanket May 2, 2026 •

edited

Loading

Rylan12 left a comment •

edited

Loading

Rylan12 commented Apr 18, 2026 •

edited

Loading

ooye-sanket May 2, 2026 •

edited

Loading

Rylan12 left a comment •

edited

Loading