From 2c7ef064e4ff0833ac94a4f18f5049ad17c30b7c Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Wed, 4 Oct 2017 15:47:53 +0200 Subject: [PATCH] Refactor DSL and Artifacts. --- .../lib/hbc/artifact/abstract_artifact.rb | 2 +- .../lib/hbc/artifact/abstract_flight_block.rb | 6 ----- Library/Homebrew/cask/lib/hbc/audit.rb | 2 +- .../Homebrew/cask/lib/hbc/container/base.rb | 2 +- Library/Homebrew/cask/lib/hbc/dsl.rb | 19 ++++++++-------- Library/Homebrew/cask/lib/hbc/dsl/appcast.rb | 16 +++++++------- Library/Homebrew/cask/lib/hbc/dsl/base.rb | 6 ++--- Library/Homebrew/cask/lib/hbc/staged.rb | 2 +- Library/Homebrew/test/cask/dsl_spec.rb | 22 +++++++++---------- 9 files changed, 35 insertions(+), 42 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb index 1b18cc6e7d..fa8fc29968 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_artifact.rb @@ -20,7 +20,7 @@ module Hbc end def self.for_cask(cask) - cask.artifacts[dsl_key].to_a + cask.artifacts[self].to_a end # TODO: this sort of logic would make more sense in dsl.rb, or a diff --git a/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb b/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb index 4e8edbc11c..a3075ff409 100644 --- a/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb +++ b/Library/Homebrew/cask/lib/hbc/artifact/abstract_flight_block.rb @@ -11,12 +11,6 @@ module Hbc dsl_key.to_s.prepend("uninstall_").to_sym end - def self.for_cask(cask) - [dsl_key, uninstall_dsl_key].flat_map do |key| - [*cask.artifacts[key]].map { |block| new(cask, key => block) } - end - end - attr_reader :directives def initialize(cask, **directives) diff --git a/Library/Homebrew/cask/lib/hbc/audit.rb b/Library/Homebrew/cask/lib/hbc/audit.rb index 9ab93a67f0..2d8e4e4d74 100644 --- a/Library/Homebrew/cask/lib/hbc/audit.rb +++ b/Library/Homebrew/cask/lib/hbc/audit.rb @@ -218,7 +218,7 @@ module Hbc end def check_generic_artifacts - cask.artifacts[:artifact].each do |artifact| + cask.artifacts[Hbc::Artifact::Artifact].each do |artifact| unless artifact.target.absolute? add_error "target must be absolute path for #{artifact.class.english_name} #{artifact.source}" end diff --git a/Library/Homebrew/cask/lib/hbc/container/base.rb b/Library/Homebrew/cask/lib/hbc/container/base.rb index 52eb5aab17..4363168e73 100644 --- a/Library/Homebrew/cask/lib/hbc/container/base.rb +++ b/Library/Homebrew/cask/lib/hbc/container/base.rb @@ -20,7 +20,7 @@ module Hbc unless children.count == 1 && !nested_container.directory? && - @cask.artifacts[:nested_container].empty? && + Artifact::NestedContainer.for_cask(@cask).none? && extract_nested_container(nested_container) children.each do |src| diff --git a/Library/Homebrew/cask/lib/hbc/dsl.rb b/Library/Homebrew/cask/lib/hbc/dsl.rb index 8d5b6c9a9d..75c1ca8736 100644 --- a/Library/Homebrew/cask/lib/hbc/dsl.rb +++ b/Library/Homebrew/cask/lib/hbc/dsl.rb @@ -43,7 +43,7 @@ module Hbc Artifact::Zap, ].freeze - ACTIVATABLE_ARTIFACT_TYPES = (ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key) - [:stage_only]).freeze + ACTIVATABLE_ARTIFACT_CLASSES = ORDINARY_ARTIFACT_CLASSES - [Artifact::StageOnly] ARTIFACT_BLOCK_CLASSES = [ Artifact::PreflightBlock, @@ -71,11 +71,12 @@ module Hbc :version, :appdir, *ORDINARY_ARTIFACT_CLASSES.map(&:dsl_key), - *ACTIVATABLE_ARTIFACT_TYPES, + *ACTIVATABLE_ARTIFACT_CLASSES.map(&:dsl_key), *ARTIFACT_BLOCK_CLASSES.flat_map { |klass| [klass.dsl_key, klass.uninstall_dsl_key] }, ].freeze - attr_reader :token, :cask + attr_reader :cask, :token + def initialize(cask) @cask = cask @token = cask.token @@ -175,7 +176,7 @@ module Hbc DSL::Container.new(*args).tap do |container| # TODO: remove this backward-compatibility section after removing nested_container if container&.nested - artifacts[:nested_container] << Artifact::NestedContainer.new(cask, container.nested) + artifacts[Artifact::NestedContainer] << Artifact::NestedContainer.new(cask, container.nested) end end end @@ -250,15 +251,13 @@ module Hbc end ORDINARY_ARTIFACT_CLASSES.each do |klass| - type = klass.dsl_key - - define_method(type) do |*args| + define_method(klass.dsl_key) do |*args| begin - if [*artifacts.keys, type].include?(:stage_only) && (artifacts.keys & ACTIVATABLE_ARTIFACT_TYPES).any? + if [*artifacts.keys, klass].include?(Artifact::StageOnly) && (artifacts.keys & ACTIVATABLE_ARTIFACT_CLASSES).any? raise CaskInvalidError.new(cask, "'stage_only' must be the only activatable artifact.") end - artifacts[type].add(klass.from_args(cask, *args)) + artifacts[klass].add(klass.from_args(cask, *args)) rescue CaskInvalidError raise rescue StandardError => e @@ -270,7 +269,7 @@ module Hbc ARTIFACT_BLOCK_CLASSES.each do |klass| [klass.dsl_key, klass.uninstall_dsl_key].each do |dsl_key| define_method(dsl_key) do |&block| - artifacts[dsl_key] << block + artifacts[klass] << klass.new(cask, dsl_key => block) end end end diff --git a/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb b/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb index f984ac088c..f3994b81f2 100644 --- a/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb +++ b/Library/Homebrew/cask/lib/hbc/dsl/appcast.rb @@ -3,17 +3,17 @@ require "hbc/system_command" module Hbc class DSL class Appcast - attr_reader :parameters, :checkpoint + attr_reader :uri, :checkpoint, :parameters - def initialize(uri, parameters = {}) - @parameters = parameters - @uri = URI(uri) - @checkpoint = @parameters[:checkpoint] + def initialize(uri, **parameters) + @uri = URI(uri) + @parameters = parameters + @checkpoint = parameters[:checkpoint] end def calculate_checkpoint curl_executable, *args = curl_args( - "--compressed", "--location", "--fail", @uri, + "--compressed", "--location", "--fail", uri, user_agent: :fake ) result = SystemCommand.run(curl_executable, args: args, print_stderr: false) @@ -30,11 +30,11 @@ module Hbc end def to_yaml - [@uri, @parameters].to_yaml + [uri, parameters].to_yaml end def to_s - @uri.to_s + uri.to_s end end end diff --git a/Library/Homebrew/cask/lib/hbc/dsl/base.rb b/Library/Homebrew/cask/lib/hbc/dsl/base.rb index 63df847e95..b97c4e104e 100644 --- a/Library/Homebrew/cask/lib/hbc/dsl/base.rb +++ b/Library/Homebrew/cask/lib/hbc/dsl/base.rb @@ -10,14 +10,14 @@ module Hbc def_delegators :@cask, :token, :version, :caskroom_path, :staged_path, :appdir, :language - def system_command(executable, options = {}) - @command.run!(executable, options) + def system_command(executable, **options) + @command.run!(executable, **options) end def method_missing(method, *) if method underscored_class = self.class.name.gsub(/([[:lower:]])([[:upper:]][[:lower:]])/, '\1_\2').downcase - section = underscored_class.downcase.split("::").last + section = underscored_class.split("::").last Utils.method_missing_message(method, @cask.to_s, section) nil else diff --git a/Library/Homebrew/cask/lib/hbc/staged.rb b/Library/Homebrew/cask/lib/hbc/staged.rb index dc21279dee..6fd3f6709a 100644 --- a/Library/Homebrew/cask/lib/hbc/staged.rb +++ b/Library/Homebrew/cask/lib/hbc/staged.rb @@ -4,7 +4,7 @@ module Hbc index = 0 if index == :first index = 1 if index == :second index = -1 if index == :last - @cask.artifacts[:app].to_a.at(index).target.join("Contents", "Info.plist") + @cask.artifacts[Artifact::App].to_a.at(index).target.join("Contents", "Info.plist") end def plist_exec(cmd) diff --git a/Library/Homebrew/test/cask/dsl_spec.rb b/Library/Homebrew/test/cask/dsl_spec.rb index 7df8de6f81..e03af9aef4 100644 --- a/Library/Homebrew/test/cask/dsl_spec.rb +++ b/Library/Homebrew/test/cask/dsl_spec.rb @@ -216,12 +216,12 @@ describe Hbc::DSL, :cask do app "Bar.app" end - expect(cask.artifacts[:app].map(&:to_s)).to eq(["Foo.app (App)", "Bar.app (App)"]) + expect(cask.artifacts[Hbc::Artifact::App].map(&:to_s)).to eq(["Foo.app (App)", "Bar.app (App)"]) end it "allow app stanzas to be empty" do cask = Hbc::Cask.new("cask-with-no-apps") - expect(cask.artifacts[:app]).to be_empty + expect(cask.artifacts[Hbc::Artifact::App]).to be_empty end end @@ -249,7 +249,7 @@ describe Hbc::DSL, :cask do pkg "Bar.pkg" end - expect(cask.artifacts[:pkg].map(&:to_s)).to eq(["Foo.pkg (Pkg)", "Bar.pkg (Pkg)"]) + expect(cask.artifacts[Hbc::Artifact::Pkg].map(&:to_s)).to eq(["Foo.pkg (Pkg)", "Bar.pkg (Pkg)"]) end end @@ -501,10 +501,10 @@ describe Hbc::DSL, :cask do let(:token) { "with-installer-script" } it "allows installer script to be specified" do - expect(cask.artifacts[:installer].first.path).to eq(Pathname("/usr/bin/true")) - expect(cask.artifacts[:installer].first.args[:args]).to eq(["--flag"]) - expect(cask.artifacts[:installer].to_a[1].path).to eq(Pathname("/usr/bin/false")) - expect(cask.artifacts[:installer].to_a[1].args[:args]).to eq(["--flag"]) + expect(cask.artifacts[Hbc::Artifact::Installer].first.path).to eq(Pathname("/usr/bin/true")) + expect(cask.artifacts[Hbc::Artifact::Installer].first.args[:args]).to eq(["--flag"]) + expect(cask.artifacts[Hbc::Artifact::Installer].to_a[1].path).to eq(Pathname("/usr/bin/false")) + expect(cask.artifacts[Hbc::Artifact::Installer].to_a[1].args[:args]).to eq(["--flag"]) end end @@ -512,7 +512,7 @@ describe Hbc::DSL, :cask do let(:token) { "with-installer-manual" } it "allows installer manual to be specified" do - installer = cask.artifacts[:installer].first + installer = cask.artifacts[Hbc::Artifact::Installer].first expect(installer).to be_a(Hbc::Artifact::Installer::ManualInstaller) expect(installer.path).to eq(cask.staged_path.join("Caffeine.app")) end @@ -524,7 +524,7 @@ describe Hbc::DSL, :cask do let(:token) { "stage-only" } it "allows stage_only stanza to be specified" do - expect(cask.artifacts[:stage_only]).not_to be_empty + expect(cask.artifacts[Hbc::Artifact::StageOnly]).not_to be_empty end end @@ -550,7 +550,7 @@ describe Hbc::DSL, :cask do let(:token) { "appdir-interpolation" } it "is allowed" do - expect(cask.artifacts[:binary].first.source).to eq(Hbc.appdir/"some/path") + expect(cask.artifacts[Hbc::Artifact::Binary].first.source).to eq(Hbc.appdir/"some/path") end end @@ -563,7 +563,7 @@ describe Hbc::DSL, :cask do binary "#{appdir}/some/path" end - expect(cask.artifacts[:binary].first.source).to eq(original_appdir/"some/path") + expect(cask.artifacts[Hbc::Artifact::Binary].first.source).to eq(original_appdir/"some/path") ensure Hbc.appdir = original_appdir end