Skip to content
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

Tweak cask-source API handling #14439

Merged
merged 6 commits into from Feb 2, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions Library/Homebrew/api.rb
Expand Up @@ -3,7 +3,6 @@

require "api/analytics"
require "api/cask"
require "api/cask-source"
require "api/formula"
require "api/versions"
require "extend/cachable"
Expand All @@ -23,23 +22,20 @@ module API
HOMEBREW_CACHE_API = (HOMEBREW_CACHE/"api").freeze
MAX_RETRIES = 3

sig { params(endpoint: String, json: T::Boolean).returns(T.any(String, Hash)) }
def fetch(endpoint, json: true)
sig { params(endpoint: String).returns(Hash) }
def fetch(endpoint)
return cache[endpoint] if cache.present? && cache.key?(endpoint)

api_url = "#{API_DOMAIN}/#{endpoint}"
output = Utils::Curl.curl_output("--fail", api_url, max_time: 5)
raise ArgumentError, "No file found at #{Tty.underline}#{api_url}#{Tty.reset}" unless output.success?

cache[endpoint] = if json
JSON.parse(output.stdout)
else
output.stdout
end
cache[endpoint] = JSON.parse(output.stdout)
rescue JSON::ParserError
raise ArgumentError, "Invalid JSON file: #{Tty.underline}#{api_url}#{Tty.reset}"
end

sig { params(endpoint: String, target: Pathname).returns(Hash) }
def fetch_json_api_file(endpoint, target:)
retry_count = 0

Expand All @@ -58,5 +54,19 @@ def fetch_json_api_file(endpoint, target:)
retry
end
end

sig { params(filepath: String, repo: String, git_head: T.nilable(String)).returns(String) }
def fetch_file_source(filepath, repo:, git_head: nil)
git_head ||= "master"
endpoint = "#{git_head}/#{filepath}"
return cache[endpoint] if cache.present? && cache.key?(endpoint)

raw_url = "https://raw.githubusercontent.com/#{repo}/#{endpoint}"
puts "Fetching #{raw_url}..."
output = Utils::Curl.curl_output("--fail", raw_url, max_time: 5)
raise ArgumentError, "No file found at #{Tty.underline}#{raw_url}#{Tty.reset}" unless output.success?

cache[endpoint] = output.stdout
end
end
end
29 changes: 0 additions & 29 deletions Library/Homebrew/api/cask-source.rb

This file was deleted.

11 changes: 8 additions & 3 deletions Library/Homebrew/api/cask.rb
Expand Up @@ -10,9 +10,14 @@ module Cask
class << self
extend T::Sig

sig { params(name: String).returns(Hash) }
def fetch(name)
Homebrew::API.fetch "cask/#{name}.json"
sig { params(token: String).returns(Hash) }
def fetch(token)
Homebrew::API.fetch "cask/#{token}.json"
end

sig { params(token: String, git_head: T.nilable(String)).returns(String) }
def fetch_source(token, git_head: nil)
Homebrew::API.fetch_file_source "Casks/#{token}.rb", repo: "Homebrew/homebrew-cask", git_head: git_head
end

sig { returns(Hash) }
Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/cask/cask.rb
Expand Up @@ -246,6 +246,7 @@ def to_h
"conflicts_with" => conflicts_with,
"container" => container&.pairs,
"auto_updates" => auto_updates,
"tap_git_head" => tap&.git_head,
}
end

Expand Down
10 changes: 6 additions & 4 deletions Library/Homebrew/cask/cask_loader.rb
Expand Up @@ -212,17 +212,19 @@ def load(config:)

json_cask.deep_symbolize_keys!

# Use the cask-source API if there are any `*flight` blocks
# Download and use the cask source file 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)
cask_source = Homebrew::API::Cask.fetch_source(token, git_head: json_cask[:tap_git_head])
return FromContentLoader.new(cask_source).load(config: config)
end

# convert generic string replacements into actual ones
json_cask[:artifacts] = json_cask[:artifacts].map(&method(:from_h_hash_gsubs))
json_cask[:caveats] = from_h_string_gsubs(json_cask[:caveats])

Cask.new(token, source: cask_source, config: config) do
tap = Tap.fetch(json_cask[:tap]) if json_cask[:tap].to_s.include?("/")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the case where json_cask[:tap].to_s does not include /?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mainly thinking of the case where it's an empty/invalid string.


Cask.new(token, tap: tap, source: cask_source, config: config) do
version json_cask[:version]

if json_cask[:sha256] == "no_check"
Expand Down Expand Up @@ -370,7 +372,7 @@ def self.for(ref, need_path: false)
return loader_class.new(ref)
end

if Homebrew::EnvConfig.install_from_api? && !need_path && Homebrew::API::CaskSource.available?(ref)
if Homebrew::EnvConfig.install_from_api? && !need_path && Homebrew::API::Cask.all_casks.key?(ref)
return FromAPILoader.new(ref)
end

Expand Down
1 change: 1 addition & 0 deletions Library/Homebrew/formula.rb
Expand Up @@ -2126,6 +2126,7 @@ def to_hash
"disabled" => disabled?,
"disable_date" => disable_date,
"disable_reason" => disable_reason,
"tap_git_head" => tap&.git_head,
}

if stable
Expand Down
8 changes: 6 additions & 2 deletions Library/Homebrew/system_config.rb
Expand Up @@ -141,11 +141,15 @@ def describe_curl

def core_tap_config(f = $stdout)
if CoreTap.instance.installed?
f.puts "Core tap ORIGIN: #{core_tap_origin}"
f.puts "Core tap origin: #{core_tap_origin}"
f.puts "Core tap HEAD: #{core_tap_head}"
f.puts "Core tap last commit: #{core_tap_last_commit}"
f.puts "Core tap branch: #{core_tap_branch}"
else
end

if (formula_json = Homebrew::API::HOMEBREW_CACHE_API/"formula.json") && formula_json.exist?
f.puts "Core tap JSON: #{formula_json.mtime.to_s(:short)}"
elsif !CoreTap.instance.installed?
f.puts "Core tap: N/A"
end
end
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/tap.rb
Expand Up @@ -178,7 +178,7 @@ def git_branch
def git_head
raise TapUnavailableError, name unless installed?

path.git_head
@git_head ||= path.git_head
end

# Time since last git commit for this {Tap}.
Expand Down
10 changes: 10 additions & 0 deletions Library/Homebrew/test/api/cask_spec.rb
Expand Up @@ -41,4 +41,14 @@ def mock_curl_download(stdout:)
expect(casks_output).to eq casks_hash
end
end

describe "::fetch_source" do
it "fetches the source of a cask (defaulting to master when no `git_head` is passed)" do
curl_output = OpenStruct.new(stdout: "foo", success?: true)
expect(Utils::Curl).to receive(:curl_output)
.with("--fail", "https://raw.githubusercontent.com/Homebrew/homebrew-cask/master/Casks/foo.rb", max_time: 5)
.and_return(curl_output)
described_class.fetch_source("foo", git_head: nil)
end
end
end
21 changes: 15 additions & 6 deletions Library/Homebrew/test/api_spec.rb
Expand Up @@ -21,12 +21,6 @@ def mock_curl_download(stdout:)
end

describe "::fetch" do
it "fetches a text file" do
mock_curl_output stdout: text
fetched_text = described_class.fetch("foo.txt", json: false)
expect(fetched_text).to eq text
end

it "fetches a JSON file" do
mock_curl_output stdout: json
fetched_json = described_class.fetch("foo.json")
Expand Down Expand Up @@ -70,4 +64,19 @@ def mock_curl_download(stdout:)
}.to raise_error(SystemExit)
end
end

describe "::fetch_file_source" do
it "fetches a file" do
mock_curl_output stdout: json
fetched_json = described_class.fetch_file_source("foo.json", repo: "Homebrew/homebrew-core", git_head: "master")
expect(fetched_json).to eq json
end

it "raises an error if the file does not exist" do
mock_curl_output success: false
expect {
described_class.fetch_file_source("bar.txt", repo: "Homebrew/homebrew-core", git_head: "master")
}.to raise_error(ArgumentError, /No file found/)
end
end
end