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

Delay loading from cask source api #14820

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
42 changes: 31 additions & 11 deletions Library/Homebrew/cask/cask.rb
Original file line number Diff line number Diff line change
Expand Up @@ -83,15 +83,14 @@ def tap
@tap
end

def initialize(token, sourcefile_path: nil, source: nil, source_checksum: nil, tap: nil,
config: nil, allow_reassignment: false, loaded_from_api: false, loader: nil, &block)
def initialize(token, sourcefile_path: nil, source: nil, tap: nil,
config: nil, allow_reassignment: false, loader: nil, &block)
@token = token
@sourcefile_path = sourcefile_path
@source = source
@ruby_source_checksum = source_checksum
@tap = tap
@allow_reassignment = allow_reassignment
@loaded_from_api = loaded_from_api
@loaded_from_api = false
@loader = loader
@block = block

Expand Down Expand Up @@ -166,6 +165,12 @@ def installed?
!versions.empty?
end

# The caskfile is needed during installation when there are
# `*flight` blocks or the cask has multiple languages
def caskfile_only?
languages.any? || artifacts.any?(Artifact::AbstractFlightBlock)
end

sig { returns(T.nilable(Time)) }
def install_time
_, time = timestamped_versions.last
Expand Down Expand Up @@ -258,6 +263,27 @@ def outdated_info(greedy, verbose, json, greedy_latest, greedy_auto_updates)
end
end

def ruby_source_checksum
@ruby_source_checksum ||= {
"sha256" => Digest::SHA256.file(sourcefile_path).hexdigest,
}.freeze
end

def languages
@languages ||= @dsl.languages
end

def tap_git_head
@tap_git_head ||= tap&.git_head
end

def populate_from_api!(json_cask)
@loaded_from_api = true
@languages = json_cask[:languages]
@tap_git_head = json_cask[:tap_git_head]
@ruby_source_checksum = json_cask[:ruby_source_checksum].freeze
end

def to_s
@token
end
Expand Down Expand Up @@ -306,7 +332,7 @@ def to_h
"conflicts_with" => conflicts_with,
"container" => container&.pairs,
"auto_updates" => auto_updates,
"tap_git_head" => tap&.git_head,
"tap_git_head" => tap_git_head,
"languages" => languages,
"ruby_source_checksum" => ruby_source_checksum,
}
Expand Down Expand Up @@ -360,12 +386,6 @@ def api_to_local_hash(hash)
hash
end

def ruby_source_checksum
@ruby_source_checksum ||= {
"sha256" => Digest::SHA256.file(sourcefile_path).hexdigest,
}
end

def artifacts_list
artifacts.map do |artifact|
case artifact
Expand Down
48 changes: 13 additions & 35 deletions Library/Homebrew/cask/cask_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,7 @@ def load(config:)
private

def cask(header_token, **options, &block)
checksum = {
"sha256" => Digest::SHA256.hexdigest(content),
}
Cask.new(header_token, source: content, source_checksum: checksum, tap: tap, **options,
config: @config, &block)
Cask.new(header_token, source: content, tap: tap, **options, config: @config, &block)
end
end

Expand Down Expand Up @@ -150,18 +146,10 @@ def self.can_load?(ref)
super && !Tap.from_path(ref).nil?
end

attr_reader :tap

def initialize(path)
@tap = Tap.from_path(path)
super(path)
end

private

def cask(*args, &block)
super(*args, tap: tap, &block)
end
Comment on lines -153 to -164
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can remove these parts because @tap is now defined in the FromContentLoader and passed into the cask constructor from there.

Easy to verify by doing the following. Note that the FromDefaultTapPathLoader inherits from the FromTapPathLoader.

/u/l/Homebrew (delay-loading-from-cask-source-api|) $ brew irb
==> Interactive Homebrew Shell
Example commands available with: `brew irb --examples`
irb(main):001:0> cask = Cask::CaskLoader::FromDefaultTapPathLoader.new("virtualbox").load(config: nil); nil
=> nil
irb(main):002:0> pp cask.tap; nil
#<Tap:0x00007f9ff2a13530
 @alias_reverse_table=nil,
 @alias_table=nil,
 @cask_dir=
  #<Pathname:/usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Casks>,
 @full_name="Homebrew/homebrew-cask",
 @name="homebrew/cask",
 @path=#<Pathname:/usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask>,
 @repo="cask",
 @user="Homebrew">
=> nil

end

# Loads a cask from a specific tap.
Expand Down Expand Up @@ -214,8 +202,6 @@ def load(config:)
class FromAPILoader
attr_reader :token, :path

FLIGHT_STANZAS = [:preflight, :postflight, :uninstall_preflight, :uninstall_postflight].freeze

def self.can_load?(ref)
return false if Homebrew::EnvConfig.no_install_from_api?
return false unless ref.is_a?(String)
Expand All @@ -235,31 +221,17 @@ def load(config:)
json_cask = @from_json || Homebrew::API::Cask.all_casks[token]
cask_source = JSON.pretty_generate(json_cask)

json_cask = Homebrew::API.merge_variations(json_cask).deep_symbolize_keys
tap = Tap.fetch(json_cask[:tap]) if json_cask[:tap].to_s.include?("/")
json_cask = Homebrew::API.merge_variations(json_cask).deep_symbolize_keys.freeze
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just a nice to have thing.


# Use the cask-source API if there are any `*flight` blocks or the cask has multiple languages
if json_cask[:artifacts].any? { |artifact| FLIGHT_STANZAS.include?(artifact.keys.first) } ||
json_cask[:languages].any?
cask_source = Homebrew::API::Cask.fetch_source(token,
git_head: json_cask[:tap_git_head],
sha256: json_cask.dig(:ruby_source_checksum, :sha256))
return FromContentLoader.new(cask_source, tap: tap).load(config: config)
end
tap = Tap.fetch(json_cask[:tap]) if json_cask[:tap].to_s.include?("/")

user_agent = json_cask.dig(:url_specs, :user_agent)
json_cask[:url_specs][:user_agent] = user_agent[1..].to_sym if user_agent && user_agent[0] == ":"
if (using = json_cask.dig(:url_specs, :using))
json_cask[:url_specs][:using] = using.to_sym
end

Cask.new(token,
tap: tap,
source: cask_source,
source_checksum: json_cask[:ruby_source_checksum],
config: config,
loaded_from_api: true,
loader: self) do
api_cask = Cask.new(token, tap: tap, source: cask_source, config: config, loader: self) do
version json_cask[:version]

if json_cask[:sha256] == "no_check"
Expand Down Expand Up @@ -319,15 +291,21 @@ def load(config:)
# convert generic string replacements into actual ones
artifact = cask.loader.from_h_hash_gsubs(artifact, appdir)
key = artifact.keys.first
send(key, *artifact[key])
if artifact[key].nil?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe it'd be better to be explicit here but at the moment the only artifacts with null values are *flight blocks.

# for artifacts with blocks that can't be loaded from the API
send(key) {} # empty on purpose
else
send(key, *artifact[key])
end
end

if json_cask[:caveats].present?
# convert generic string replacements into actual ones
json_cask[:caveats] = cask.loader.from_h_string_gsubs(json_cask[:caveats], appdir)
caveats json_cask[:caveats]
caveats cask.loader.from_h_string_gsubs(json_cask[:caveats], appdir)
end
end
api_cask.populate_from_api!(json_cask)
api_cask
end

def from_h_string_gsubs(string, appdir)
Expand Down
1 change: 0 additions & 1 deletion Library/Homebrew/cask/dsl.rb
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,6 @@ class DSL
:depends_on,
:homepage,
:language,
:languages,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This now gets defined explicitly in the cask class.

:name,
:sha256,
:staged_path,
Expand Down
39 changes: 30 additions & 9 deletions Library/Homebrew/cask/installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ def self.caveats(cask)
def fetch(quiet: nil, timeout: nil)
odebug "Cask::Installer#fetch"

load_cask_from_source_api! if @cask.loaded_from_api && @cask.caskfile_only?

verify_has_sha if require_sha? && !force?

download(quiet: quiet, timeout: timeout)
Expand Down Expand Up @@ -149,16 +151,8 @@ def reinstall
def uninstall_existing_cask
return unless @cask.installed?

# use the same cask file that was used for installation, if possible
installed_caskfile = @cask.installed_caskfile
installed_cask = begin
installed_caskfile.exist? ? CaskLoader.load(installed_caskfile) : @cask
rescue CaskInvalidError # could be thrown by call to CaskLoader#load with outdated caskfile
@cask # default
end

# Always force uninstallation, ignore method parameter
cask_installer = Installer.new(installed_cask, verbose: verbose?, force: true, upgrade: upgrade?)
cask_installer = Installer.new(@cask, verbose: verbose?, force: true, upgrade: upgrade?)
zap? ? cask_installer.zap : cask_installer.uninstall
end

Expand Down Expand Up @@ -403,6 +397,7 @@ def save_download_sha
end

def uninstall
load_installed_caskfile!
oh1 "Uninstalling Cask #{Formatter.identifier(@cask)}"
uninstall_artifacts(clear: true)
if !reinstall? && !upgrade?
Expand Down Expand Up @@ -480,6 +475,7 @@ def uninstall_artifacts(clear: false)
end

def zap
load_installed_caskfile!
ohai "Implied `brew uninstall --cask #{@cask}`"
uninstall_artifacts
if (zap_stanzas = @cask.artifacts.select { |a| a.is_a?(Artifact::Zap) }).empty?
Expand Down Expand Up @@ -547,5 +543,30 @@ def purge_caskroom_path
odebug "Purging all staged versions of Cask #{@cask}"
gain_permissions_remove(@cask.caskroom_path)
end

private

# load the same cask file that was used for installation, if possible
def load_installed_caskfile!
installed_caskfile = @cask.installed_caskfile

if installed_caskfile.exist?
begin
@cask = CaskLoader.load(installed_caskfile)
return
rescue CaskInvalidError
# could be caused by trying to load outdated caskfile
end
end

load_cask_from_source_api! if @cask.loaded_from_api && @cask.caskfile_only?
# otherwise we default to the current cask
end

def load_cask_from_source_api!
options = { git_head: @cask.tap_git_head, sha256: @cask.ruby_source_checksum["sha256"] }
cask_source = Homebrew::API::Cask.fetch_source(@cask.token, **options)
@cask = CaskLoader::FromContentLoader.new(cask_source, tap: @cask.tap).load(config: @cask.config)
end
end
end
76 changes: 30 additions & 46 deletions Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,80 +59,64 @@
end

describe "#load" do
shared_examples "loads from fetched source" do |cask_token|
include_context "with API setup", cask_token
let(:content_loader) { instance_double(Cask::CaskLoader::FromContentLoader) }

it "fetches cask source from API" do
expect(Homebrew::API::Cask).to receive(:fetch_source).once
expect(Cask::CaskLoader::FromContentLoader)
.to receive(:new).once
.and_return(content_loader)
expect(content_loader).to receive(:load).once

api_loader.load(config: nil)
end
end

context "with a preflight stanza" do
include_examples "loads from fetched source", "with-preflight"
end

context "with an uninstall-preflight stanza" do
include_examples "loads from fetched source", "with-uninstall-preflight"
end

context "with a postflight stanza" do
include_examples "loads from fetched source", "with-postflight"
end

context "with an uninstall-postflight stanza" do
include_examples "loads from fetched source", "with-uninstall-postflight"
end

context "with a language stanza" do
include_examples "loads from fetched source", "with-languages"
end

shared_examples "loads from API" do |cask_token|
shared_examples "loads from API" do |cask_token, caskfile_only|
include_context "with API setup", cask_token
let(:cask_from_api) { api_loader.load(config: nil) }

it "loads from JSON API" do
expect(Homebrew::API::Cask).not_to receive(:fetch_source)
expect(Cask::CaskLoader::FromContentLoader).not_to receive(:new)

expect(cask_from_api).to be_a(Cask::Cask)
expect(cask_from_api.token).to eq(cask_token)
expect(cask_from_api.loaded_from_api).to be(true)
expect(cask_from_api.caskfile_only?).to be(caskfile_only)
end
end

context "with a binary stanza" do
include_examples "loads from API", "with-binary"
include_examples "loads from API", "with-binary", false
end

context "with cask dependencies" do
include_examples "loads from API", "with-depends-on-cask-multiple"
include_examples "loads from API", "with-depends-on-cask-multiple", false
end

context "with formula dependencies" do
include_examples "loads from API", "with-depends-on-formula-multiple"
include_examples "loads from API", "with-depends-on-formula-multiple", false
end

context "with macos dependencies" do
include_examples "loads from API", "with-depends-on-macos-array"
include_examples "loads from API", "with-depends-on-macos-array", false
end

context "with an installer stanza" do
include_examples "loads from API", "with-installer-script"
include_examples "loads from API", "with-installer-script", false
end

context "with uninstall stanzas" do
include_examples "loads from API", "with-uninstall-multi"
include_examples "loads from API", "with-uninstall-multi", false
end

context "with a zap stanza" do
include_examples "loads from API", "with-zap"
include_examples "loads from API", "with-zap", false
end

context "with a preflight stanza" do
include_examples "loads from API", "with-preflight", true
end

context "with an uninstall-preflight stanza" do
include_examples "loads from API", "with-uninstall-preflight", true
end

context "with a postflight stanza" do
include_examples "loads from API", "with-postflight", true
end

context "with an uninstall-postflight stanza" do
include_examples "loads from API", "with-uninstall-postflight", true
end

context "with a language stanza" do
include_examples "loads from API", "with-languages", true
end
end
end