Conversation
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
left a comment
MikeMcQuaid
left a comment
There was a problem hiding this comment.
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.
| rescue JSON::ParserError | ||
| [] |
There was a problem hiding this comment.
Probably worth a opoo warning of the exception in this case?
|
Thanks @MikeMcQuaid! I've pushed the changes:
Let me know if the level of inlining looks right or if you'd like it structured differently! |
MikeMcQuaid
left a comment
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks! Please ensure CI syntax job passing before asking for re-review.
|
|
||
| sig { override.void } | ||
| def run | ||
| db_path = args.output_db&.then { |path| Pathname(path) } |
There was a problem hiding this comment.
Not sure I understand the .then usage here: can you explain?
| formula = Formulary.factory(name) | ||
| line = db_line(formula) | ||
| if db_path | ||
| existing = db_path.exist? ? db_path.readlines(chomp: true).reject(&:empty?) : [] |
There was a problem hiding this comment.
| 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
| 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}(") } |
There was a problem hiding this comment.
Could maybe drop existing as a variable and just chain this?
| 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 |
| else | ||
| puts line if line |
There was a problem hiding this comment.
| else | |
| puts line if line | |
| elsif line | |
| puts line |
| lines = db_path.readlines(chomp: true).reject { |l| l.start_with?("#{name}(") } | ||
| db_path.write("#{lines.sort.join("\n")}\n") |
There was a problem hiding this comment.
This logic is repeated, maybe worth turning into a function or restructuring
2 participants
Adds
brew which-entry <formula>a developer command that generatesa single
executables.txtentry for a formula by readingsh.brew.path_exec_filesfrom its bottle manifest.This is intended to replace the batch
brew which-updatejob with aper-formula update triggered by homebrew/core CI on each merge.
What it does
sh.brew.path_exec_files--output-db=<file>: upserts the formula's line in the DB fileand removes it if the formula is disabled, deprecated, or deleted
--output-db=: prints the DB line to stdoutRelationship to other PRs
The companion homebrew/core PR (Homebrew/homebrew-core#272976) slims
the existing workflow down to just call
brew which-entryper formuladetected by
brew test-bot --only-formulae-detect, replacing thecurrent daily batch job entirely.
@MikeMcQuaid please have a look!
brew lgtm(style, typechecking and tests) with your changes locally?