From fd18c7b0ac04dfbbdd005facf806b0bc4a438c75 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 26 Jan 2023 17:36:40 +0000 Subject: [PATCH 1/5] Tweak cask-source API handling - Use raw.githubusercontent.com to download cask source rather than formulae.brew.sh. This allows us to remove these files - output the tap's current `HEAD` for both formulae and cask JSON - use this `HEAD` for the cask-source API to get the exact file on raw.githubusercontent.com rather than just whatever is newest (which is what the previous API did) - set the `Tap` correctly when creating a `Cask` from the API - if the `formula.json` file exists: print its modified time include `brew config` - memoize `tap.git_head` as we'll be calling it a lot in the same process with the same value --- Library/Homebrew/api.rb | 24 +++++++++++++++++------- Library/Homebrew/api/cask-source.rb | 6 +++--- Library/Homebrew/cask/cask.rb | 1 + Library/Homebrew/cask/cask_loader.rb | 6 ++++-- Library/Homebrew/formula.rb | 1 + Library/Homebrew/system_config.rb | 8 ++++++-- Library/Homebrew/tap.rb | 2 +- 7 files changed, 33 insertions(+), 15 deletions(-) diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index a6dadf6fa6..fefdd421fe 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -23,23 +23,20 @@ module Homebrew 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 @@ -58,5 +55,18 @@ module Homebrew retry end end + + sig { params(token: String, git_head: T.nilable(String)).returns(String) } + def fetch_source(token, git_head: nil) + git_head ||= "master" + endpoint = "#{git_head}/Casks/#{token}.rb" + return cache[endpoint] if cache.present? && cache.key?(endpoint) + + raw_url = "https://raw.githubusercontent.com/Homebrew/homebrew-cask/#{endpoint}" + 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 diff --git a/Library/Homebrew/api/cask-source.rb b/Library/Homebrew/api/cask-source.rb index 0c1cd67a21..e0c6296c3f 100644 --- a/Library/Homebrew/api/cask-source.rb +++ b/Library/Homebrew/api/cask-source.rb @@ -10,10 +10,10 @@ module Homebrew class << self extend T::Sig - sig { params(token: String).returns(Hash) } - def fetch(token) + sig { params(token: String, git_head: T.nilable(String)).returns(Hash) } + def fetch(token, git_head: nil) token = token.sub(%r{^homebrew/cask/}, "") - Homebrew::API.fetch "cask-source/#{token}.rb", json: false + Homebrew::API.fetch_source token, git_head: git_head end sig { params(token: String).returns(T::Boolean) } diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 2e00a20a70..9e75ac77a2 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -246,6 +246,7 @@ module Cask "conflicts_with" => conflicts_with, "container" => container&.pairs, "auto_updates" => auto_updates, + "tap_git_head" => tap&.git_head, } end diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 6e66b043ee..ad60176eb7 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -214,7 +214,7 @@ module Cask # 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) + cask_source = Homebrew::API::CaskSource.fetch(token, git_head: json_cask[:tap_git_head]) return FromContentLoader.new(cask_source).load(config: config) end @@ -222,7 +222,9 @@ module Cask 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?("/") + + Cask.new(token, tap: tap, source: cask_source, config: config) do version json_cask[:version] if json_cask[:sha256] == "no_check" diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb index bb8a0e6bfd..aaf6edb60f 100644 --- a/Library/Homebrew/formula.rb +++ b/Library/Homebrew/formula.rb @@ -2126,6 +2126,7 @@ class Formula "disabled" => disabled?, "disable_date" => disable_date, "disable_reason" => disable_reason, + "tap_git_head" => tap&.git_head, } if stable diff --git a/Library/Homebrew/system_config.rb b/Library/Homebrew/system_config.rb index a019aa4de0..9f1d969ca5 100644 --- a/Library/Homebrew/system_config.rb +++ b/Library/Homebrew/system_config.rb @@ -141,11 +141,15 @@ module SystemConfig 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 diff --git a/Library/Homebrew/tap.rb b/Library/Homebrew/tap.rb index a6aed47cbd..5c3433cbd8 100644 --- a/Library/Homebrew/tap.rb +++ b/Library/Homebrew/tap.rb @@ -178,7 +178,7 @@ class Tap 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}. From 32a0877cad27ec49cf08b26c1bc53349cf4d63f6 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 28 Jan 2023 02:15:00 -0600 Subject: [PATCH 2/5] Remove `CaskSource` API --- Library/Homebrew/api.rb | 8 ++++---- Library/Homebrew/api/cask-source.rb | 29 ---------------------------- Library/Homebrew/api/cask.rb | 11 ++++++++--- Library/Homebrew/cask/cask_loader.rb | 6 +++--- 4 files changed, 15 insertions(+), 39 deletions(-) delete mode 100644 Library/Homebrew/api/cask-source.rb diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index fefdd421fe..b365bdd1f0 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -3,7 +3,6 @@ require "api/analytics" require "api/cask" -require "api/cask-source" require "api/formula" require "api/versions" require "extend/cachable" @@ -57,12 +56,13 @@ module Homebrew end sig { params(token: String, git_head: T.nilable(String)).returns(String) } - def fetch_source(token, git_head: nil) + def fetch_file_source(filepath, repo:, git_head: nil) git_head ||= "master" - endpoint = "#{git_head}/Casks/#{token}.rb" + endpoint = "#{git_head}/#{filepath}" return cache[endpoint] if cache.present? && cache.key?(endpoint) - raw_url = "https://raw.githubusercontent.com/Homebrew/homebrew-cask/#{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? diff --git a/Library/Homebrew/api/cask-source.rb b/Library/Homebrew/api/cask-source.rb deleted file mode 100644 index e0c6296c3f..0000000000 --- a/Library/Homebrew/api/cask-source.rb +++ /dev/null @@ -1,29 +0,0 @@ -# typed: false -# frozen_string_literal: true - -module Homebrew - module API - # Helper functions for using the cask source API. - # - # @api private - module CaskSource - class << self - extend T::Sig - - sig { params(token: String, git_head: T.nilable(String)).returns(Hash) } - def fetch(token, git_head: nil) - token = token.sub(%r{^homebrew/cask/}, "") - Homebrew::API.fetch_source token, git_head: git_head - end - - sig { params(token: String).returns(T::Boolean) } - def available?(token) - fetch token - true - rescue ArgumentError - false - end - end - end - end -end diff --git a/Library/Homebrew/api/cask.rb b/Library/Homebrew/api/cask.rb index f0ed248cdf..4f7c4b8e36 100644 --- a/Library/Homebrew/api/cask.rb +++ b/Library/Homebrew/api/cask.rb @@ -10,9 +10,14 @@ module Homebrew 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) } diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index ad60176eb7..79f494a3bc 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -212,9 +212,9 @@ module Cask 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, git_head: json_cask[:tap_git_head]) + cask_source = Homebrew::API::Cask.fetch_source(token, git_head: json_cask[:tap_git_head]) return FromContentLoader.new(cask_source).load(config: config) end @@ -372,7 +372,7 @@ module Cask 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 From 014b603b1f7c4731e1de0ebd27355f186438e285 Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 28 Jan 2023 02:15:05 -0600 Subject: [PATCH 3/5] Fix tests --- Library/Homebrew/test/api/cask_spec.rb | 10 ++++++++++ Library/Homebrew/test/api_spec.rb | 21 +++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/test/api/cask_spec.rb b/Library/Homebrew/test/api/cask_spec.rb index 35c561b8f9..b5dd7ec1f2 100644 --- a/Library/Homebrew/test/api/cask_spec.rb +++ b/Library/Homebrew/test/api/cask_spec.rb @@ -41,4 +41,14 @@ describe Homebrew::API::Cask do 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 diff --git a/Library/Homebrew/test/api_spec.rb b/Library/Homebrew/test/api_spec.rb index ff695bfe77..6d639e5c0f 100644 --- a/Library/Homebrew/test/api_spec.rb +++ b/Library/Homebrew/test/api_spec.rb @@ -21,12 +21,6 @@ describe Homebrew::API do 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") @@ -70,4 +64,19 @@ describe Homebrew::API do }.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 From 2c78840f57af7773f7b3ba324ecc58781bc0a64f Mon Sep 17 00:00:00 2001 From: Rylan Polster Date: Sat, 28 Jan 2023 02:16:09 -0600 Subject: [PATCH 4/5] Fix typecheck issue --- Library/Homebrew/api.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/api.rb b/Library/Homebrew/api.rb index b365bdd1f0..49bb507b1d 100644 --- a/Library/Homebrew/api.rb +++ b/Library/Homebrew/api.rb @@ -55,7 +55,7 @@ module Homebrew end end - sig { params(token: String, git_head: T.nilable(String)).returns(String) } + 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}" From 1c1c1623e24eab2c51b004db1a21561f080c9c9d Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Thu, 2 Feb 2023 12:19:44 +0000 Subject: [PATCH 5/5] test/cask/cmd/list: add tap_git_head. --- Library/Homebrew/test/cask/cmd/list_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/Library/Homebrew/test/cask/cmd/list_spec.rb b/Library/Homebrew/test/cask/cmd/list_spec.rb index 8d397e19f4..3c1518cafc 100644 --- a/Library/Homebrew/test/cask/cmd/list_spec.rb +++ b/Library/Homebrew/test/cask/cmd/list_spec.rb @@ -128,6 +128,7 @@ describe Cask::Cmd::List, :cask do "conflicts_with": null, "container": null, "auto_updates": null, + "tap_git_head": null, "languages": [ ] @@ -162,6 +163,7 @@ describe Cask::Cmd::List, :cask do "conflicts_with": null, "container": null, "auto_updates": null, + "tap_git_head": null, "languages": [ ] @@ -199,6 +201,7 @@ describe Cask::Cmd::List, :cask do "conflicts_with": null, "container": null, "auto_updates": null, + "tap_git_head": null, "languages": [ ] @@ -233,6 +236,7 @@ describe Cask::Cmd::List, :cask do "conflicts_with": null, "container": null, "auto_updates": null, + "tap_git_head": null, "languages": [ ] @@ -267,6 +271,7 @@ describe Cask::Cmd::List, :cask do "conflicts_with": null, "container": null, "auto_updates": null, + "tap_git_head": null, "languages": [ "zh", "en-US"