Allow brew info --json=v2 without taps with JSON API
#14587
Conversation
Rylan12
added
critical
|
Review period skipped due to |
There was a problem hiding this comment.
While this is possible: I'm not sure it's a good idea to introduce a circular dependency here. We did that already on formulae.brew.sh and it was a bit of a nightmare to resolve.
Not opposed to adding this as-is but: let's just make sure we literally never can even attempt to generate formulae.brew.sh data from this.
|
Yeah, good point. How about this as a long-term plan: Make a new dev command called like |
|
@Rylan12 yup, sounds great 👍🏻. happy to ship this as-is. |
| if loaded_from_api && Homebrew::EnvConfig.install_from_api? | ||
| json_cask = Homebrew::API::Cask.all_casks[token] | ||
| return Homebrew::API.merge_variations(json_cask) | ||
| end |
There was a problem hiding this comment.
I'd rather not have to add this code to each of the #to_h* methods. Is this functionally necessary or a performance improvement? Put another way we'll have already loaded the cask/formula from the API at this point; will the normal #to_h* method return the expected output?
There was a problem hiding this comment.
If it's a performance optimization, it'd be nice to have benchmarks. Also, if it's only relevant for brew info I'd rather add the code there instead of to the #to_h* methods.
There was a problem hiding this comment.
Put another way we'll have already loaded the cask/formula from the API at this point; will the normal
#to_h*method return the expected output?
The original implementation was non-functional: #14541
There was a problem hiding this comment.
Ah okay, that was the context I was missing. Looks like the *flight blocks are still a thorn in our side when it comes to the cask API and that's really what needs to be fixed eventually but I guess this is fine as a temporary fix.
Since we aren't using the *flight block info right now when installing from the API (we download the source instead) we could probably just revert back to the previous behavior instead of bundling each block as a string. It might actually solve this problem. I could probably get a PR related to that up by tomorrow.
There was a problem hiding this comment.
Since we aren't using the *flight block info right now when installing from the API (we download the source instead) we could probably just revert back to the previous behavior instead of bundling each block as a string. It might actually solve this problem. I could probably get a PR related to that up by tomorrow.
@apainintheneck This sounds good 👍🏻
|
This needs a rebase btw. |
MikeMcQuaid
force-pushed
the
info-json-api
branch
from
e89eb51
to
f7fa593
Compare
We should allow
brew info --json=v2even without the taps installed. We can just pass the downloaded JSON file along without needing to regerate the data using#to_hon the formulae and casks. I've tested this locally without homebrew/core or homebrew/cask and it seems to work with an without--variations, with--installed, and with--all --eval-all.One thing to note: when you run
brew info --jsonit will merge in the correct OS variations which I think makes the most sense since someone running that command probably cares about what's currently on their system.brew info --json --variations, however, will return the complete with-variations JSON file. This is consistent with the way things work withHOMEBREW_NO_INSTALL_FROM_API.Also, while we're at it, let's fully remove the bottle API. It's already been removed from formulae.brew.sh and the
--bottleswitch to turn it on was hidden. I don't think we'll need to do any deprecations for it but we should revert that part if anyone complains.