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:10:05Z

+
+      sig { override.void }
+      def run
+        db_path = args.output_db&.then { |path| Pathname(path) }


Not sure I understand the .then usage here: can you explain?

MikeMcQuaid · 2026-03-22T14:10:57Z

+        formula = Formulary.factory(name)
+        line = db_line(formula)
+        if db_path
+          existing = db_path.exist? ? db_path.readlines(chomp: true).reject(&:empty?) : []


Suggested change

existing = db_path.exist? ? db_path.readlines(chomp: true).reject(&:empty?) : []

existing = db_path.readlines(chomp: true).reject(&:blank?) if db_path.exist?

existing ||= []

maybe

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

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


Could maybe drop existing as a variable and just chain this?

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?

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

+        else
+          puts line if line


Suggested change

else

puts line if line

elsif line

puts line

MikeMcQuaid · 2026-03-22T14:12:07Z

+        lines = db_path.readlines(chomp: true).reject { |l| l.start_with?("#{name}(") }
+        db_path.write("#{lines.sort.join("\n")}\n")


This logic is repeated, maybe worth turning into a function or restructuring

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.

When marking a formula as not installed on request, also set installed_as_dependency to true, to ensure brew bundle dump correctly excludes it. Previously, brew tab --no-installed-on-request had no effect on bundle dump because the dump logic checks both Fixes Homebrew#21751

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.

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}(") }

MikeMcQuaid · 2026-04-14T13:49:39Z

+      def executables_from_manifest(formula)
+        return [] unless formula.bottled?
+
+        manifest_path = HOMEBREW_CACHE.glob("#{formula.name}_bottle_manifest--*").first
+        if manifest_path.blank?
+          bottle = formula.bottle
+          return [] unless bottle
+
+          bottle.fetch
+          manifest_path = HOMEBREW_CACHE.glob("#{formula.name}_bottle_manifest--*").first
+        end
+        return [] if manifest_path.blank?
+
+        manifest = JSON.parse(manifest_path.read)
+        exec_files = manifest.dig("manifests", 0, "annotations", "sh.brew.path_exec_files")
+        return [] if exec_files.blank?
+
+        exec_files.split.map { |f| File.basename(f) }.sort
+      rescue JSON::ParserError => e
+        opoo "Failed to parse bottle manifest for #{formula.name}: #{e.message}"
+        []
+      end


can this share any logic or constants with the relevant bottle.rb or resource.rb code?

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

Rylan12 · 2026-04-17T00:53:24Z

+        return if formula.disabled? || formula.deprecated?
+
+        exes = executables_from_manifest(formula)
+        "#{formula.full_name}(#{formula.pkg_version}):#{exes.join(" ")}"


The reason we added pkg_version before was to track formula file changes over time (we only checked the formulae with changed versions for updated exes).

Now that we're going to run this on every merge where we can detect which formulae are updated, I don't think we need the pkg_version anymore

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 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 added 7 commits April 13, 2026 08:48

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

a3c0d9b

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

ae66e65

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

6c1b0ab

improve as address by maintainer

32a99d3

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

eb11c44

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

00d9e78

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

	existing = db_path.exist? ? db_path.readlines(chomp: true).reject(&:empty?) : []
	existing = db_path.readlines(chomp: true).reject(&:blank?) if db_path.exist?
	existing \|\|= []

		lines = db_path.readlines(chomp: true).reject { \|l\| l.start_with?("#{name}(") }
		db_path.write("#{lines.sort.join("\n")}\n")

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

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!

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!

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!

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!

Choose a reason for hiding this comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Rylan12 left a comment •

edited

Loading

Rylan12 commented Apr 18, 2026 •

edited

Loading