-
-
Notifications
You must be signed in to change notification settings - Fork 10.1k
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
Load casks from the JSON API with HOMEBREW_INSTALL_FROM_API
#14304
Changes from all commits
374b615
bab85d8
08529c3
68bbe03
54e013a
52263e2
ffc74a5
ec8adb6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -186,6 +186,100 @@ def load(config:) | |||||
end | ||||||
end | ||||||
|
||||||
# Loads a cask from the JSON API. | ||||||
class FromAPILoader | ||||||
attr_reader :token, :path | ||||||
|
||||||
FLIGHT_STANZAS = [:preflight, :postflight, :uninstall_preflight, :uninstall_postflight].freeze | ||||||
|
||||||
def self.can_load?(ref) | ||||||
Homebrew::API::Cask.all_casks.key? ref | ||||||
end | ||||||
|
||||||
def initialize(token) | ||||||
@token = token | ||||||
@path = CaskLoader.default_path(token) | ||||||
end | ||||||
|
||||||
def load(config:) | ||||||
json_cask = Homebrew::API::Cask.all_casks[token] | ||||||
|
||||||
if (bottle_tag = ::Utils::Bottles.tag.to_s.presence) && | ||||||
(variations = json_cask["variations"].presence) && | ||||||
(variation = variations[bottle_tag].presence) | ||||||
json_cask = json_cask.merge(variation) | ||||||
end | ||||||
|
||||||
json_cask.deep_symbolize_keys! | ||||||
|
||||||
# Use the cask-source API if there are any `*flight` blocks | ||||||
if json_cask[:artifacts].any? { |artifact| FLIGHT_STANZAS.include?(artifact.keys.first) } | ||||||
cask_source = Homebrew::API::CaskSource.fetch(token) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Not for this PR: I think it would be good for this API to require fetching the cask source at a particular Git SHA/revision that matches that from the JSON, for consistency and to be able to use the e.g. https://raw.githubusercontent.com/Homebrew/homebrew-cask/25e4143c41a98abb50cde6a5042e957fe5988236/Casks/0-ad.rb endpoints. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we originally opted to host our own source files on formulae.brew.sh because we felt that it would be faster than the files GitHub was hosting. Now that we're only using this method for a small number of casks, should we switch to using the GitHub-provided source URLs with hashes? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Rylan12 Yeh, I think so. Some advantages:
|
||||||
return FromContentLoader.new(cask_source).load(config: config) | ||||||
end | ||||||
|
||||||
Cask.new(token, source: cask_source, config: config) do | ||||||
version json_cask[:version] | ||||||
|
||||||
if json_cask[:sha256] == "no_check" | ||||||
sha256 :no_check | ||||||
else | ||||||
sha256 json_cask[:sha256] | ||||||
end | ||||||
|
||||||
url json_cask[:url] | ||||||
appcast json_cask[:appcast] if json_cask[:appcast].present? | ||||||
json_cask[:name].each do |cask_name| | ||||||
name cask_name | ||||||
end | ||||||
desc json_cask[:desc] | ||||||
homepage json_cask[:homepage] | ||||||
|
||||||
auto_updates json_cask[:auto_updates] if json_cask[:auto_updates].present? | ||||||
conflicts_with(**json_cask[:conflicts_with]) if json_cask[:conflicts_with].present? | ||||||
|
||||||
if json_cask[:depends_on].present? | ||||||
dep_hash = json_cask[:depends_on].to_h do |dep_key, dep_value| | ||||||
next [dep_key, dep_value] unless dep_key == :macos | ||||||
|
||||||
dep_type = dep_value.keys.first | ||||||
if dep_type == :== | ||||||
version_symbols = dep_value[dep_type].map do |version| | ||||||
MacOSVersions::SYMBOLS.key(version) || version | ||||||
end | ||||||
next [dep_key, version_symbols] | ||||||
end | ||||||
|
||||||
version_symbol = dep_value[dep_type].first | ||||||
version_symbol = MacOSVersions::SYMBOLS.key(version_symbol) || version_symbol | ||||||
[dep_key, "#{dep_type} :#{version_symbol}"] | ||||||
end.compact | ||||||
depends_on(**dep_hash) | ||||||
end | ||||||
|
||||||
if json_cask[:container].present? | ||||||
container_hash = json_cask[:container].to_h do |container_key, container_value| | ||||||
next [container_key, container_value] unless container_key == :type | ||||||
|
||||||
[container_key, container_value.to_sym] | ||||||
end | ||||||
container(**container_hash) | ||||||
end | ||||||
|
||||||
json_cask[:artifacts].each do |artifact| | ||||||
key = artifact.keys.first | ||||||
if FLIGHT_STANZAS.include?(key) | ||||||
instance_eval(artifact[key]) | ||||||
else | ||||||
send(key, *artifact[key]) | ||||||
end | ||||||
end | ||||||
|
||||||
caveats json_cask[:caveats] if json_cask[:caveats].present? | ||||||
end | ||||||
end | ||||||
end | ||||||
|
||||||
# Pseudo-loader which raises an error when trying to load the corresponding cask. | ||||||
class NullLoader < FromPathLoader | ||||||
extend T::Sig | ||||||
|
@@ -225,15 +319,15 @@ def self.for(ref, need_path: false) | |||||
next unless loader_class.can_load?(ref) | ||||||
|
||||||
if loader_class == FromTapLoader && Homebrew::EnvConfig.install_from_api? && | ||||||
ref.start_with?("homebrew/cask/") && Homebrew::API::CaskSource.available?(ref) | ||||||
return FromContentLoader.new(Homebrew::API::CaskSource.fetch(ref)) | ||||||
ref.start_with?("homebrew/cask/") && FromAPILoader.can_load?(ref) | ||||||
return FromAPILoader.new(ref) | ||||||
end | ||||||
|
||||||
return loader_class.new(ref) | ||||||
end | ||||||
|
||||||
if Homebrew::EnvConfig.install_from_api? && !need_path && Homebrew::API::CaskSource.available?(ref) | ||||||
return FromContentLoader.new(Homebrew::API::CaskSource.fetch(ref)) | ||||||
return FromAPILoader.new(ref) | ||||||
end | ||||||
|
||||||
return FromTapPathLoader.new(default_path(ref)) if FromTapPathLoader.can_load?(default_path(ref)) | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
# typed: false | ||
# frozen_string_literal: true | ||
|
||
require "api" | ||
|
||
describe Homebrew::API::Cask do | ||
let(:cache_dir) { mktmpdir } | ||
|
||
before do | ||
stub_const("Homebrew::API::HOMEBREW_CACHE_API", cache_dir) | ||
end | ||
|
||
def mock_curl_download(stdout:) | ||
allow(Utils::Curl).to receive(:curl_download) do |*_args, **kwargs| | ||
kwargs[:to].write stdout | ||
end | ||
end | ||
|
||
describe "::all_casks" do | ||
let(:casks_json) { | ||
<<~EOS | ||
[{ | ||
"token": "foo", | ||
"url": "https://brew.sh/foo" | ||
}, { | ||
"token": "bar", | ||
"url": "https://brew.sh/bar" | ||
}] | ||
EOS | ||
} | ||
let(:casks_hash) { | ||
{ | ||
"foo" => { "url" => "https://brew.sh/foo" }, | ||
"bar" => { "url" => "https://brew.sh/bar" }, | ||
} | ||
} | ||
|
||
it "returns the expected cask JSON list" do | ||
mock_curl_download stdout: casks_json | ||
casks_output = described_class.all_casks | ||
expect(casks_output).to eq casks_hash | ||
end | ||
end | ||
end |
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.
Not for this PR: I wonder if it might be nice to push this to the API instead/as well so we have something like
needs_cask_source: true
in the JSON?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.
Do you think we want to remove the
*flight
block source that was added in #14324 from the API if we're not actually going to use it?