New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
items.sh: return early when no Taps directory #14259
Conversation
Review period will end on 2022-12-19 at 21:44:38 UTC. |
6112f31
to
04a61fb
Compare
Review period ended. |
a90b67a
to
c679d06
Compare
Library/Homebrew/cmd/formulae.sh
Outdated
local api_formulae | ||
api_formulae="$(ruby -e "require 'json'; JSON.parse(File.read('${HOMEBREW_CACHE}/api/formula.json')).each { |f| puts f['name'] }")" | ||
formulae=$(echo -e "${formulae}\n${api_formulae}" | sort -uf | grep .) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept idea for reading cached JSON using ruby without brew libraries.
This currently doesn't handle any error inside Ruby. Particularly, a corrupt JSON file ends up dumping the entire contents of formula.json to terminal which can be quite lengthy. EDIT: hiding stderr should be enough for this.
I had added grep .
since ${formulae}
can be ""
so it leads to blank line. Not sure if there is better way to handle this.
c679d06
to
a0e9352
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This makes sense! Happy to try it out.
if [[ -n "${HOMEBREW_INSTALL_FROM_API}" && -f "${HOMEBREW_CACHE}/api/formula.json" ]] | ||
then | ||
local api_formulae | ||
api_formulae="$(ruby -e "require 'json'; JSON.parse(File.read('${HOMEBREW_CACHE}/api/formula.json')).each { |f| puts f['name'] }" 2>/dev/null)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should do this calculation when we download HOMEBREW_CACHE/api/formula.json
, instead. That way we only need to do it once per unique cache file (I'm assuming this line is relatively slow)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Rylan12 That seems like a good idea! Let's merge as-is for now and iterate to improve performance here 💪🏻
Thanks again @cho-m! |
brew style
with your changes locally?brew typecheck
with your changes locally?brew tests
with your changes locally?Workaround for removing errors output in #14254. Will need API usage for actual completion support, but haven't had chance to look into this.
EDIT: Updated to allow completion from cached JSON to fix #14254
Haven't tested on
HOMEBREW_INSTALL_FROM_API
setup, but seemed to work when I changed Taps in following line to invalid directory:brew/Library/Homebrew/items.sh
Line 21 in 884c4be
e.g. changing above to
Taps2
and trying without this PR:with PR: