Merge pull request #2825 from reitermarkus/refactoring

Refactoring using `Forwardable` and `DelegateClass`.
This commit is contained in:
Markus Reiter 2017-07-01 05:32:19 +02:00 committed by GitHub
commit fd5e673d2d
30 changed files with 207 additions and 289 deletions

View File

@ -1,6 +1,8 @@
module Hbc
module Artifact
class Base
extend Predicable
def self.artifact_name
@artifact_name ||= name.sub(/^.*:/, "").gsub(/(.)([A-Z])/, '\1_\2').downcase
end
@ -65,13 +67,7 @@ module Hbc
{}
end
def verbose?
@verbose
end
def force?
@force
end
attr_predicate :force?, :verbose?
def initialize(cask, command: SystemCommand, force: false, verbose: false)
@cask = cask

View File

@ -1,5 +1,3 @@
require "forwardable"
require "hbc/dsl"
require "hbc/metadata"
@ -73,6 +71,15 @@ module Hbc
@token
end
def hash
token.hash
end
def eql?(other)
token == other.token
end
alias == eql?
def dumpcask
odebug "Cask instance dumps in YAML:"
odebug "Cask instance toplevel:", to_yaml

View File

@ -1,35 +1,36 @@
require "delegate"
require "hbc/topological_hash"
module Hbc
class CaskDependencies
attr_reader :cask, :graph, :sorted
class CaskDependencies < DelegateClass(Array)
attr_reader :cask, :graph
def initialize(cask)
@cask = cask
@graph = graph_dependencies
@sorted = sort
super(sort)
end
def graph_dependencies
deps_in = ->(csk) { csk.depends_on ? csk.depends_on.cask || [] : [] }
walk = lambda do |acc, deps|
deps.each do |dep|
next if acc.key?(dep)
succs = deps_in.call CaskLoader.load(dep)
acc[dep] = succs
walk.call(acc, succs)
end
acc
end
private
graphed = walk.call({}, @cask.depends_on.cask)
TopologicalHash[graphed]
def graph_dependencies(cask = self.cask, acc = TopologicalHash.new)
return acc if acc.key?(cask)
deps = cask.depends_on.cask.map(&CaskLoader.public_method(:load))
acc[cask] = deps
deps.each do |dep|
graph_dependencies(dep, acc)
end
acc
end
def sort
@graph.tsort
raise CaskSelfReferencingDependencyError, cask.token if graph[cask].include?(cask)
graph.tsort - [cask]
rescue TSort::Cyclic
raise CaskCyclicCaskDependencyError, @cask.token
strongly_connected_components = graph.strongly_connected_components.sort_by(&:count)
cyclic_dependencies = strongly_connected_components.last - [cask]
raise CaskCyclicDependencyError.new(cask.token, cyclic_dependencies.join(", "))
end
end
end

View File

@ -1,12 +1,14 @@
module Hbc
module CaskLoader
class FromContentLoader
attr_reader :content
def initialize(content)
@content = content
end
def load
instance_eval(@content.force_encoding("UTF-8"), __FILE__, __LINE__)
instance_eval(content.force_encoding("UTF-8"), __FILE__, __LINE__)
end
private
@ -18,25 +20,25 @@ module Hbc
class FromPathLoader < FromContentLoader
def self.can_load?(ref)
path = Pathname.new(ref)
path = Pathname(ref)
path.extname == ".rb" && path.expand_path.exist?
end
attr_reader :token, :path
def initialize(path)
path = Pathname.new(path).expand_path
path = Pathname(path).expand_path
@token = path.basename(".rb").to_s
@path = path
end
def load
raise CaskUnavailableError.new(@token, "'#{@path}' does not exist.") unless @path.exist?
raise CaskUnavailableError.new(@token, "'#{@path}' is not readable.") unless @path.readable?
raise CaskUnavailableError.new(@token, "'#{@path}' is not a file.") unless @path.file?
raise CaskUnavailableError.new(token, "'#{path}' does not exist.") unless path.exist?
raise CaskUnavailableError.new(token, "'#{path}' is not readable.") unless path.readable?
raise CaskUnavailableError.new(token, "'#{path}' is not a file.") unless path.file?
@content = IO.read(@path)
@content = IO.read(path)
super
end
@ -44,11 +46,11 @@ module Hbc
private
def cask(header_token, &block)
if @token != header_token
raise CaskTokenMismatchError.new(@token, header_token)
if token != header_token
raise CaskTokenMismatchError.new(token, header_token)
end
Cask.new(header_token, sourcefile_path: @path, &block)
Cask.new(header_token, sourcefile_path: path, &block)
end
end
@ -65,14 +67,13 @@ module Hbc
end
def load
Hbc.cache.mkpath
FileUtils.rm_f @path
path.dirname.mkpath
begin
ohai "Downloading #{@url}."
curl @url, "-o", @path
ohai "Downloading #{url}."
curl url, "-o", path
rescue ErrorDuringExecution
raise CaskUnavailableError.new(@token, "Failed to download #{Formatter.url(@url)}.")
raise CaskUnavailableError.new(token, "Failed to download #{Formatter.url(url)}.")
end
super
@ -84,15 +85,17 @@ module Hbc
ref.to_s.match?(HOMEBREW_TAP_CASK_REGEX)
end
attr_reader :tap
def initialize(tapped_name)
user, repo, token = tapped_name.split("/", 3).map(&:downcase)
user, repo, token = tapped_name.split("/", 3)
@tap = Tap.fetch(user, repo)
super @tap.cask_dir/"#{token}.rb"
end
def load
@tap.install unless @tap.installed?
tap.install unless tap.installed?
super
end
@ -104,12 +107,12 @@ module Hbc
end
def initialize(ref)
@token = File.basename(ref, ".rb")
super CaskLoader.default_path(@token)
token = File.basename(ref, ".rb")
super CaskLoader.default_path(token)
end
def load
raise CaskUnavailableError.new(@token, "No Cask with this name exists.")
raise CaskUnavailableError.new(token, "No Cask with this name exists.")
end
end

View File

@ -1,29 +1,27 @@
module Hbc
module Checkable
def errors
Array(@errors)
@errors ||= []
end
def warnings
Array(@warnings)
@warnings ||= []
end
def add_error(message)
@errors ||= []
@errors << message
errors << message
end
def add_warning(message)
@warnings ||= []
@warnings << message
warnings << message
end
def errors?
Array(@errors).any?
errors.any?
end
def warnings?
Array(@warnings).any?
warnings.any?
end
def result

View File

@ -100,7 +100,7 @@ module Hbc
end
def _fetch
odebug "Calling curl with args #{cask_curl_args.utf8_inspect}"
odebug "Calling curl with args #{cask_curl_args}"
curl(*cask_curl_args)
end

View File

@ -211,10 +211,10 @@ module Hbc
# depends_on uses a load method so that multiple stanzas can be merged
def depends_on(*args)
return @depends_on if args.empty?
@depends_on ||= DSL::DependsOn.new
return @depends_on if args.empty?
begin
@depends_on.load(*args) unless args.empty?
@depends_on.load(*args)
rescue RuntimeError => e
raise CaskInvalidError.new(token, e)
end

View File

@ -24,6 +24,8 @@ module Hbc
def initialize
@pairs ||= {}
@cask ||= []
@formula ||= []
end
def load(pairs = {})
@ -53,12 +55,10 @@ module Hbc
end
def formula=(*args)
@formula ||= []
@formula.concat(args)
end
def cask=(*args)
@cask ||= []
@cask.concat(args)
end

View File

@ -77,9 +77,15 @@ module Hbc
end
end
class CaskCyclicCaskDependencyError < AbstractCaskErrorWithToken
class CaskCyclicDependencyError < AbstractCaskErrorWithToken
def to_s
"Cask '#{token}' includes cyclic dependencies on other Casks and could not be installed."
"Cask '#{token}' includes cyclic dependencies on other Casks" << (reason.empty? ? "." : ": #{reason}")
end
end
class CaskSelfReferencingDependencyError < CaskCyclicDependencyError
def to_s
"Cask '#{token}' depends on itself."
end
end
@ -91,7 +97,7 @@ module Hbc
class CaskInvalidError < AbstractCaskErrorWithToken
def to_s
"Cask '#{token}' definition is invalid" << (reason.empty? ? ".": ": #{reason}")
"Cask '#{token}' definition is invalid" << (reason.empty? ? "." : ": #{reason}")
end
end

View File

@ -1,11 +1,14 @@
require "rubygems"
require "formula_installer"
require "hbc/cask_dependencies"
require "hbc/staged"
require "hbc/verify"
module Hbc
class Installer
extend Predicable
# TODO: it is unwise for Hbc::Staged to be a module, when we are
# dealing with both staged and unstaged Casks here. This should
# either be a class which is only sometimes instantiated, or there
@ -27,21 +30,7 @@ module Hbc
@reinstall = false
end
def skip_cask_deps?
@skip_cask_deps
end
def force?
@force
end
def binaries?
@binaries
end
def verbose?
@verbose
end
attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, :verbose?
def self.print_caveats(cask)
odebug "Printing caveats"
@ -75,7 +64,7 @@ module Hbc
odebug "Hbc::Installer#fetch"
satisfy_dependencies
verify_has_sha if @require_sha && !force?
verify_has_sha if require_sha? && !force?
download
verify
end
@ -210,7 +199,6 @@ module Hbc
x11_dependencies
formula_dependencies
cask_dependencies unless skip_cask_deps?
puts "complete"
end
def macos_dependencies
@ -247,36 +235,50 @@ module Hbc
end
def formula_dependencies
return unless @cask.depends_on.formula && !@cask.depends_on.formula.empty?
ohai "Installing Formula dependencies from Homebrew"
@cask.depends_on.formula.each do |dep_name|
print "#{dep_name} ... "
installed = @command.run(HOMEBREW_BREW_FILE,
args: ["list", "--versions", dep_name],
print_stderr: false).stdout.include?(dep_name)
if installed
puts "already installed"
else
@command.run!(HOMEBREW_BREW_FILE,
args: ["install", dep_name])
puts "done"
formulae = @cask.depends_on.formula.map { |f| Formula[f] }
return if formulae.empty?
if formulae.all?(&:any_version_installed?)
puts "All Formula dependencies satisfied."
return
end
not_installed = formulae.reject(&:any_version_installed?)
ohai "Installing Formula dependencies: #{not_installed.map(&:to_s).join(", ")}"
not_installed.each do |formula|
begin
old_argv = ARGV.dup
ARGV.replace([])
FormulaInstaller.new(formula).tap do |fi|
fi.installed_as_dependency = true
fi.installed_on_request = false
fi.show_header = true
fi.verbose = verbose?
fi.prelude
fi.install
fi.finish
end
ensure
ARGV.replace(old_argv)
end
end
end
def cask_dependencies
return unless @cask.depends_on.cask && !@cask.depends_on.cask.empty?
ohai "Installing Cask dependencies: #{@cask.depends_on.cask.join(", ")}"
deps = CaskDependencies.new(@cask)
deps.sorted.each do |dep_token|
puts "#{dep_token} ..."
dep = CaskLoader.load(dep_token)
if dep.installed?
puts "already installed"
else
Installer.new(dep, binaries: binaries?, verbose: verbose?, skip_cask_deps: true, force: false).install
puts "done"
end
return if @cask.depends_on.cask.empty?
casks = CaskDependencies.new(@cask)
if casks.all?(&:installed?)
puts "All Cask dependencies satisfied."
return
end
not_installed = casks.reject(&:installed?)
ohai "Installing Cask dependencies: #{not_installed.map(&:to_s).join(", ")}"
not_installed.each do |cask|
Installer.new(cask, binaries: binaries?, verbose: verbose?, skip_cask_deps: true, force: false).install
end
end

View File

@ -20,7 +20,7 @@ module Hbc
def run!
@processed_output = { stdout: "", stderr: "" }
odebug "Executing: #{expanded_command.utf8_inspect}"
odebug "Executing: #{expanded_command}"
each_output_line do |type, line|
case type
@ -62,7 +62,7 @@ module Hbc
def assert_success
return if processed_status && processed_status.success?
raise CaskCommandFailedError.new(command.utf8_inspect, processed_output[:stdout], processed_output[:stderr], processed_status)
raise CaskCommandFailedError.new(command, processed_output[:stdout], processed_output[:stderr], processed_status)
end
def expanded_command
@ -162,7 +162,7 @@ module Hbc
raise CaskError, <<-EOS
Empty result parsing plist output from command.
command was:
#{command.utf8_inspect}
#{command}
output we attempted to parse:
#{output}
EOS

View File

@ -1,5 +1,3 @@
require "forwardable"
module Hbc
class URL
FAKE_USER_AGENT = "Mozilla/5.0 (Macintosh; Intel Mac OS X 10) https://caskroom.github.io".freeze

View File

@ -38,7 +38,7 @@ module Hbc
def _check_response_status
ok = OK_RESPONSES[cask.url.scheme]
return if ok.include?(@response_status)
add_error "unexpected http response, expecting #{ok.map(&:utf8_inspect).join(" or ")}, got #{@response_status.utf8_inspect}"
add_error "unexpected http response, expecting #{ok.map(&:to_s).join(" or ")}, got #{@response_status}"
end
def _get_data_from_request

View File

@ -4,24 +4,15 @@ require "stringio"
BUG_REPORTS_URL = "https://github.com/caskroom/homebrew-cask#reporting-bugs".freeze
# monkeypatch Object - not a great idea
class Object
def utf8_inspect
return inspect unless defined?(Encoding)
return map(&:utf8_inspect) if respond_to?(:map)
inspect.force_encoding("UTF-8").sub(/\A"(.*)"\Z/, '\1')
end
end
class Buffer < StringIO
extend Predicable
attr_predicate :tty?
def initialize(tty = false)
super()
@tty = tty
end
def tty?
@tty
end
end
# global methods

View File

@ -1,4 +1,6 @@
class Caveats
extend Forwardable
attr_reader :f
def initialize(f)
@ -25,9 +27,7 @@ class Caveats
caveats.compact.join("\n")
end
def empty?
caveats.empty?
end
delegate [:empty?, :to_s] => :caveats
private

View File

@ -1,6 +1,7 @@
class Checksum
extend Forwardable
attr_reader :hash_type, :hexdigest
alias to_s hexdigest
TYPES = [:sha256].freeze
@ -9,9 +10,7 @@ class Checksum
@hexdigest = hexdigest
end
def empty?
hexdigest.empty?
end
delegate [:empty?, :to_s] => :@hexdigest
def ==(other)
hash_type == other.hash_type && hexdigest == other.hexdigest

View File

@ -169,8 +169,8 @@ module Homebrew
Homebrew.dump_options_for_formula f
end
c = Caveats.new(f)
ohai "Caveats", c.caveats unless c.empty?
caveats = Caveats.new(f)
ohai "Caveats", caveats.to_s unless caveats.empty?
end
def decorate_dependencies(dependencies)

View File

@ -547,6 +547,8 @@ class Reporter
end
class ReporterHub
extend Forwardable
attr_reader :reporters
def initialize
@ -564,9 +566,7 @@ class ReporterHub
@hash.update(report) { |_key, oldval, newval| oldval.concat(newval) }
end
def empty?
@hash.empty?
end
delegate :empty? => :@hash
def dump
# Key Legend: Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)

View File

@ -74,18 +74,13 @@ module Debrew
end
end
class << self
alias original_raise raise
end
@active = false
@debugged_exceptions = Set.new
def self.active?
@active
end
class << self
extend Predicable
alias original_raise raise
attr_predicate :active?
attr_reader :debugged_exceptions
end

View File

@ -1,28 +1,11 @@
class Dependencies
include Enumerable
require "delegate"
def initialize
@deps = []
class Dependencies < DelegateClass(Array)
def initialize(*args)
super(args)
end
def each(*args, &block)
@deps.each(*args, &block)
end
def <<(o)
@deps << o
self
end
def empty?
@deps.empty?
end
def *(arg)
@deps * arg
end
alias to_ary to_a
alias eql? ==
def optional
select(&:optional?)
@ -44,40 +27,28 @@ class Dependencies
build + required + recommended
end
attr_reader :deps
protected :deps
def ==(other)
deps == other.deps
end
alias eql? ==
def inspect
"#<#{self.class.name}: #{to_a.inspect}>"
"#<#{self.class.name}: #{to_a}>"
end
end
class Requirements
include Enumerable
def initialize
@reqs = Set.new
end
def each(*args, &block)
@reqs.each(*args, &block)
class Requirements < DelegateClass(Set)
def initialize(*args)
super(Set.new(args))
end
def <<(other)
if other.is_a?(Comparable)
@reqs.grep(other.class) do |req|
grep(other.class) do |req|
return self if req > other
@reqs.delete(req)
delete(req)
end
end
@reqs << other
super
self
end
alias to_ary to_a
def inspect
"#<#{self.class.name}: {#{to_a.join(", ")}}>"
end
end

View File

@ -2,6 +2,7 @@ require "dependable"
# A dependency on another Homebrew formula.
class Dependency
extend Forwardable
include Dependable
attr_reader :name, :tags, :env_proc, :option_names
@ -34,9 +35,7 @@ class Dependency
formula
end
def installed?
to_formula.installed?
end
delegate installed?: :to_formula
def satisfied?(inherited_options)
installed? && missing_options(inherited_options).empty?

View File

@ -3,6 +3,7 @@ require "rexml/document"
require "time"
class AbstractDownloadStrategy
extend Forwardable
include FileUtils
attr_reader :meta, :name, :version, :resource
@ -181,9 +182,7 @@ class VCSDownloadStrategy < AbstractDownloadStrategy
@clone
end
def head?
version.head?
end
delegate head?: :version
# Return last commit's unique identifier for the repository.
# Return most recent modified timestamp unless overridden.

View File

@ -0,0 +1,9 @@
module Predicable
def attr_predicate(*attrs)
attrs.each do |attr|
define_method attr do
instance_variable_get("@#{attr.to_s.sub(/\?$/, "")}") == true
end
end
end
end

View File

@ -45,6 +45,7 @@ class Formula
include Utils::Inreplace
include Utils::Shell
extend Enumerable
extend Forwardable
# @!method inreplace(paths, before = nil, after = nil)
# Actually implemented in {Utils::Inreplace.inreplace}.
@ -314,37 +315,14 @@ class Formula
active_spec == head
end
# @private
def bottle_unneeded?
active_spec.bottle_unneeded?
end
# @private
def bottle_disabled?
active_spec.bottle_disabled?
end
# @private
def bottle_disable_reason
active_spec.bottle_disable_reason
end
# Does the currently active {SoftwareSpec} have any bottle?
# @private
def bottle_defined?
active_spec.bottle_defined?
end
# Does the currently active {SoftwareSpec} have an installable bottle?
# @private
def bottled?
active_spec.bottled?
end
# @private
def bottle_specification
active_spec.bottle_specification
end
delegate [
:bottle_unneeded?,
:bottle_disabled?,
:bottle_disable_reason,
:bottle_defined?,
:bottled?,
:bottle_specification,
] => :active_spec
# The Bottle object for the currently active {SoftwareSpec}.
# @private
@ -353,24 +331,21 @@ class Formula
end
# The description of the software.
# @method desc
# @see .desc
def desc
self.class.desc
end
delegate desc: :"self.class"
# The homepage for the software.
# @method homepage
# @see .homepage
def homepage
self.class.homepage
end
delegate homepage: :"self.class"
# The version for the currently active {SoftwareSpec}.
# The version is autodetected from the URL and/or tag so only needs to be
# declared if it cannot be autodetected correctly.
# @method version
# @see .version
def version
active_spec.version
end
delegate version: :active_spec
def update_head_version
return unless head?
@ -394,9 +369,8 @@ class Formula
# Additional downloads can be defined as {#resource}s.
# {Resource#stage} will create a temporary directory and yield to a block.
# <pre>resource("additional_files").stage { bin.install "my/extra/tool" }</pre>
def resource(name)
active_spec.resource(name)
end
# @method resource
delegate resource: :active_spec
# An old name for the formula
def oldname
@ -416,68 +390,39 @@ class Formula
end
# The {Resource}s for the currently active {SoftwareSpec}.
def resources
active_spec.resources.values
end
# @method resources
def_delegator :"active_spec.resources", :values, :resources
# The {Dependency}s for the currently active {SoftwareSpec}.
# @private
def deps
active_spec.deps
end
delegate deps: :active_spec
# The {Requirement}s for the currently active {SoftwareSpec}.
# @private
def requirements
active_spec.requirements
end
delegate requirements: :active_spec
# The cached download for the currently active {SoftwareSpec}.
# @private
def cached_download
active_spec.cached_download
end
delegate cached_download: :active_spec
# Deletes the download for the currently active {SoftwareSpec}.
# @private
def clear_cache
active_spec.clear_cache
end
delegate clear_cache: :active_spec
# The list of patches for the currently active {SoftwareSpec}.
# @private
def patchlist
active_spec.patches
end
def_delegator :active_spec, :patches, :patchlist
# The options for the currently active {SoftwareSpec}.
# @private
def options
active_spec.options
end
delegate options: :active_spec
# The deprecated options for the currently active {SoftwareSpec}.
# @private
def deprecated_options
active_spec.deprecated_options
end
delegate deprecated_options: :active_spec
# The deprecated option flags for the currently active {SoftwareSpec}.
# @private
def deprecated_flags
active_spec.deprecated_flags
end
delegate deprecated_flags: :active_spec
# If a named option is defined for the currently active {SoftwareSpec}.
def option_defined?(name)
active_spec.option_defined?(name)
end
# @method option_defined?
delegate option_defined?: :active_spec
# All the {.fails_with} for the currently active {SoftwareSpec}.
# @private
def compiler_failures
active_spec.compiler_failures
end
delegate compiler_failures: :active_spec
# If this {Formula} is installed.
# This is actually just a check for if the {#installed_prefix} directory

View File

@ -17,6 +17,7 @@ require "development_tools"
class FormulaInstaller
include FormulaCellarChecks
extend Predicable
def self.mode_attr_accessor(*names)
attr_accessor(*names)
@ -559,11 +560,11 @@ class FormulaInstaller
audit_installed if ARGV.homebrew_developer? && !formula.keg_only?
c = Caveats.new(formula)
caveats = Caveats.new(formula)
return if c.empty?
return if caveats.empty?
@show_summary_heading = true
ohai "Caveats", c.caveats
ohai "Caveats", caveats.to_s
end
def finish
@ -879,9 +880,7 @@ class FormulaInstaller
private
def hold_locks?
@hold_locks || false
end
attr_predicate :hold_locks?
def lock
return unless (@@locked ||= []).empty?

View File

@ -81,7 +81,7 @@ class BottleDisableReason
end
def to_s
if @type == :unneeded
if unneeded?
"This formula doesn't require compiling."
else
@reason

View File

@ -1,4 +1,6 @@
require "forwardable"
require "extend/module"
require "extend/predicable"
require "extend/fileutils"
require "extend/pathname"
require "extend/git_repository"

View File

@ -1,7 +1,6 @@
require "download_strategy"
require "checksum"
require "version"
require "forwardable"
# Resource is the fundamental representation of an external resource. The
# primary formula download, along with other declared resources, are instances

View File

@ -1,4 +1,3 @@
require "forwardable"
require "resource"
require "checksum"
require "version"

View File

@ -12,7 +12,7 @@ describe "Satisfy Dependencies and Requirements", :cask do
describe "depends_on cask" do
context "when depends_on cask is cyclic" do
let(:cask) { Hbc::CaskLoader.load_from_file(TEST_FIXTURE_DIR/"cask/Casks/with-depends-on-cask-cyclic.rb") }
it { is_expected.to raise_error(Hbc::CaskCyclicCaskDependencyError) }
it { is_expected.to raise_error(Hbc::CaskCyclicDependencyError) }
end
context do