From 1e1ce1c471dfaf83c8b3349a29bcb61d20ec8920 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 2 Feb 2019 17:11:37 +0100 Subject: [PATCH 1/9] Save config file for casks. --- Library/Homebrew/cask/cask.rb | 12 +- Library/Homebrew/cask/config.rb | 86 +++++------ Library/Homebrew/cask/installer.rb | 18 ++- .../test/cask/artifact/alt_target_spec.rb | 4 +- .../Homebrew/test/cask/artifact/app_spec.rb | 4 +- .../test/cask/artifact/binary_spec.rb | 4 +- .../cask/artifact/generic_artifact_spec.rb | 2 +- .../Homebrew/test/cask/artifact/suite_spec.rb | 2 +- .../cask/artifact/two_apps_correct_spec.rb | 6 +- .../Homebrew/test/cask/cmd/install_spec.rb | 11 +- Library/Homebrew/test/cask/cmd/list_spec.rb | 4 +- .../Homebrew/test/cask/cmd/uninstall_spec.rb | 8 +- .../Homebrew/test/cask/cmd/upgrade_spec.rb | 6 +- Library/Homebrew/test/cask/cmd/zap_spec.rb | 4 +- Library/Homebrew/test/cask/dsl_spec.rb | 2 +- Library/Homebrew/test/cask/installer_spec.rb | 12 +- Library/Homebrew/test/cask/quarantine_spec.rb | 138 +++++++++--------- .../spec/shared_context/homebrew_cask.rb | 2 +- 18 files changed, 171 insertions(+), 154 deletions(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 38c0ceb09a..4336cff60a 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -11,7 +11,7 @@ module Cask extend Searchable include Metadata - attr_reader :token, :sourcefile_path, :config + attr_reader :token, :sourcefile_path def self.each return to_enum unless block_given? @@ -31,7 +31,7 @@ module Cask @tap end - def initialize(token, sourcefile_path: nil, tap: nil, config: Config.global, &block) + def initialize(token, sourcefile_path: nil, tap: nil, config: nil, &block) @token = token @sourcefile_path = sourcefile_path @tap = tap @@ -77,6 +77,14 @@ module Cask metadata_master_container_path.join(*installed_version, "Casks", "#{token}.rb") end + def config + @config ||= Config.for_cask(self) + end + + def config_path + metadata_master_container_path/"dirs.json" + end + def outdated?(greedy = false) !outdated_versions(greedy).empty? end diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index 07ffee2ead..ed40737b47 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -1,12 +1,8 @@ +require "json" + module Cask - class Config - def self.global - @global ||= new - end - - attr_reader :binarydir - - def initialize( + class Config < DelegateClass(Hash) + DEFAULT_DIRS = { appdir: "/Applications", prefpanedir: "~/Library/PreferencePanes", qlplugindir: "~/Library/QuickLook", @@ -19,47 +15,51 @@ module Cask audio_unit_plugindir: "~/Library/Audio/Plug-Ins/Components", vst_plugindir: "~/Library/Audio/Plug-Ins/VST", vst3_plugindir: "~/Library/Audio/Plug-Ins/VST3", - screen_saverdir: "~/Library/Screen Savers" - ) + screen_saverdir: "~/Library/Screen Savers", + }.freeze - self.appdir = appdir - self.prefpanedir = prefpanedir - self.qlplugindir = qlplugindir - self.dictionarydir = dictionarydir - self.fontdir = fontdir - self.colorpickerdir = colorpickerdir - self.servicedir = servicedir - self.input_methoddir = input_methoddir - self.internet_plugindir = internet_plugindir - self.audio_unit_plugindir = audio_unit_plugindir - self.vst_plugindir = vst_plugindir - self.vst3_plugindir = vst3_plugindir - self.screen_saverdir = screen_saverdir - - # `binarydir` is not customisable. - @binarydir = HOMEBREW_PREFIX/"bin" + def self.global + @global ||= new end - [ - :appdir, - :prefpanedir, - :qlplugindir, - :dictionarydir, - :fontdir, - :colorpickerdir, - :servicedir, - :input_methoddir, - :internet_plugindir, - :audio_unit_plugindir, - :vst_plugindir, - :vst3_plugindir, - :screen_saverdir, - ].each do |dir| - attr_reader dir + def self.for_cask(cask) + if cask.config_path.exist? + from_file(cask.config_path) + else + global + end + end + + def self.from_file(path) + config = begin + JSON.parse(File.read(path)) + rescue JSON::ParserError => e + raise e, "Cannot parse #{path}: #{e}", e.backtrace + end + + new(Hash[config.map { |k, v| [k.to_sym, v] }]) + end + + def initialize(**dirs) + super(Hash[DEFAULT_DIRS.map { |(k, v)| [k, Pathname(dirs.fetch(k, v)).expand_path] }]) + end + + def binarydir + @binarydir ||= HOMEBREW_PREFIX/"bin" + end + + DEFAULT_DIRS.keys.each do |dir| + define_method(dir) do + self[dir] + end define_method(:"#{dir}=") do |path| - instance_variable_set(:"@#{dir}", Pathname(path).expand_path) + self[dir] = Pathname(path).expand_path end end + + def write(path) + path.atomic_write(to_json) + end end end diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 5c2b58ebd2..74f1664972 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -39,7 +39,7 @@ module Cask end attr_predicate :binaries?, :force?, :skip_cask_deps?, :require_sha?, - :upgrade?, :verbose?, :installed_as_dependency?, + :reinstall?, :upgrade?, :verbose?, :installed_as_dependency?, :quarantine? def self.print_caveats(cask) @@ -79,7 +79,7 @@ module Cask def install odebug "Cask::Installer#install" - if @cask.installed? && !force? && !@reinstall && !upgrade? + if @cask.installed? && !force? && !reinstall? && !upgrade? raise CaskAlreadyInstalledError, @cask end @@ -87,7 +87,7 @@ module Cask print_caveats fetch - uninstall_existing_cask if @reinstall + uninstall_existing_cask if reinstall? oh1 "Installing Cask #{Formatter.identifier(@cask)}" opoo "macOS's Gatekeeper has been disabled for this Cask" unless quarantine? @@ -209,6 +209,8 @@ module Cask artifact.install_phase(command: @command, verbose: verbose?, force: force?) already_installed_artifacts.unshift(artifact) end + + save_config_file rescue => e begin already_installed_artifacts.each do |artifact| @@ -382,13 +384,23 @@ module Cask old_savedir&.rmtree end + def save_config_file + @cask.config.write(@cask.config_path) + end + def uninstall oh1 "Uninstalling Cask #{Formatter.identifier(@cask)}" uninstall_artifacts(clear: true) + remove_config_file unless reinstall? || upgrade? purge_versioned_files purge_caskroom_path if force? end + def remove_config_file + FileUtils.rm_f @cask.config_path + @cask.config_path.parent.rmdir_if_possible + end + def start_upgrade oh1 "Starting upgrade for Cask #{Formatter.identifier(@cask)}" diff --git a/Library/Homebrew/test/cask/artifact/alt_target_spec.rb b/Library/Homebrew/test/cask/artifact/alt_target_spec.rb index ef929154f6..b2808c420d 100644 --- a/Library/Homebrew/test/cask/artifact/alt_target_spec.rb +++ b/Library/Homebrew/test/cask/artifact/alt_target_spec.rb @@ -11,7 +11,7 @@ describe Cask::Artifact::App, :cask do } let(:source_path) { cask.staged_path.join("Caffeine.app") } - let(:target_path) { Cask::Config.global.appdir.join("AnotherName.app") } + let(:target_path) { cask.config.appdir.join("AnotherName.app") } before do InstallHelper.install_without_artifacts(cask) @@ -58,7 +58,7 @@ describe Cask::Artifact::App, :cask do expect(target_path).to be_a_directory expect(source_path).not_to exist - expect(Cask::Config.global.appdir.join("Caffeine Deluxe.app")).not_to exist + expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist expect(cask.staged_path.join("Caffeine Deluxe.app")).to be_a_directory end diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 26abb19ad9..14917b4f42 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -5,7 +5,7 @@ describe Cask::Artifact::App, :cask do let(:app) { cask.artifacts.find { |a| a.is_a?(described_class) } } let(:source_path) { cask.staged_path.join("Caffeine.app") } - let(:target_path) { Cask::Config.global.appdir.join("Caffeine.app") } + let(:target_path) { cask.config.appdir.join("Caffeine.app") } let(:install_phase) { app.install_phase(command: command, force: force) } let(:uninstall_phase) { app.uninstall_phase(command: command, force: force) } @@ -53,7 +53,7 @@ describe Cask::Artifact::App, :cask do expect(target_path).to be_a_directory expect(source_path).not_to exist - expect(Cask::Config.global.appdir.join("Caffeine Deluxe.app")).not_to exist + expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist expect(cask.staged_path.join("Caffeine Deluxe.app")).to exist end diff --git a/Library/Homebrew/test/cask/artifact/binary_spec.rb b/Library/Homebrew/test/cask/artifact/binary_spec.rb index bd70904ebc..0aa60c38fe 100644 --- a/Library/Homebrew/test/cask/artifact/binary_spec.rb +++ b/Library/Homebrew/test/cask/artifact/binary_spec.rb @@ -5,7 +5,7 @@ describe Cask::Artifact::Binary, :cask do end } let(:artifacts) { cask.artifacts.select { |a| a.is_a?(described_class) } } - let(:expected_path) { Cask::Config.global.binarydir.join("binary") } + let(:expected_path) { cask.config.binarydir.join("binary") } after do FileUtils.rm expected_path if expected_path.exist? @@ -38,7 +38,7 @@ describe Cask::Artifact::Binary, :cask do end } - let(:expected_path) { Cask::Config.global.binarydir.join("naked_non_executable") } + let(:expected_path) { cask.config.binarydir.join("naked_non_executable") } it "makes the binary executable" do expect(FileUtils).to receive(:chmod) diff --git a/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb b/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb index ff28392175..83fb55fc58 100644 --- a/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb +++ b/Library/Homebrew/test/cask/artifact/generic_artifact_spec.rb @@ -10,7 +10,7 @@ describe Cask::Artifact::Artifact, :cask do } let(:source_path) { cask.staged_path.join("Caffeine.app") } - let(:target_path) { Cask::Config.global.appdir.join("Caffeine.app") } + let(:target_path) { cask.config.appdir.join("Caffeine.app") } before do InstallHelper.install_without_artifacts(cask) diff --git a/Library/Homebrew/test/cask/artifact/suite_spec.rb b/Library/Homebrew/test/cask/artifact/suite_spec.rb index 87f821aea2..3f4a2d6dc8 100644 --- a/Library/Homebrew/test/cask/artifact/suite_spec.rb +++ b/Library/Homebrew/test/cask/artifact/suite_spec.rb @@ -9,7 +9,7 @@ describe Cask::Artifact::Suite, :cask do end } - let(:target_path) { Cask::Config.global.appdir.join("Caffeine") } + let(:target_path) { cask.config.appdir.join("Caffeine") } let(:source_path) { cask.staged_path.join("Caffeine") } before do diff --git a/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb b/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb index a5375125ba..7381c832ba 100644 --- a/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb +++ b/Library/Homebrew/test/cask/artifact/two_apps_correct_spec.rb @@ -11,10 +11,10 @@ describe Cask::Artifact::App, :cask do } let(:source_path_mini) { cask.staged_path.join("Caffeine Mini.app") } - let(:target_path_mini) { Cask::Config.global.appdir.join("Caffeine Mini.app") } + let(:target_path_mini) { cask.config.appdir.join("Caffeine Mini.app") } let(:source_path_pro) { cask.staged_path.join("Caffeine Pro.app") } - let(:target_path_pro) { Cask::Config.global.appdir.join("Caffeine Pro.app") } + let(:target_path_pro) { cask.config.appdir.join("Caffeine Pro.app") } before do InstallHelper.install_without_artifacts(cask) @@ -52,7 +52,7 @@ describe Cask::Artifact::App, :cask do expect(target_path_mini).to be_a_directory expect(source_path_mini).not_to exist - expect(Cask::Config.global.appdir.join("Caffeine Deluxe.app")).not_to exist + expect(cask.config.appdir.join("Caffeine Deluxe.app")).not_to exist expect(cask.staged_path.join("Caffeine Deluxe.app")).to exist end diff --git a/Library/Homebrew/test/cask/cmd/install_spec.rb b/Library/Homebrew/test/cask/cmd/install_spec.rb index c3113e1662..1efe513b4c 100644 --- a/Library/Homebrew/test/cask/cmd/install_spec.rb +++ b/Library/Homebrew/test/cask/cmd/install_spec.rb @@ -21,11 +21,12 @@ describe Cask::Cmd::Install, :cask do it "allows staging and activation of multiple Casks at once" do described_class.run("local-transmission", "local-caffeine") - - expect(Cask::CaskLoader.load(cask_path("local-transmission"))).to be_installed - expect(Cask::Config.global.appdir.join("Transmission.app")).to be_a_directory - expect(Cask::CaskLoader.load(cask_path("local-caffeine"))).to be_installed - expect(Cask::Config.global.appdir.join("Caffeine.app")).to be_a_directory + transmission = Cask::CaskLoader.load(cask_path("local-transmission")) + caffeine = Cask::CaskLoader.load(cask_path("local-caffeine")) + expect(transmission).to be_installed + expect(transmission.config.appdir.join("Transmission.app")).to be_a_directory + expect(caffeine).to be_installed + expect(caffeine.config.appdir.join("Caffeine.app")).to be_a_directory end it "skips double install (without nuking existing installation)" do diff --git a/Library/Homebrew/test/cask/cmd/list_spec.rb b/Library/Homebrew/test/cask/cmd/list_spec.rb index 2c819fcb30..7d63076e46 100644 --- a/Library/Homebrew/test/cask/cmd/list_spec.rb +++ b/Library/Homebrew/test/cask/cmd/list_spec.rb @@ -80,9 +80,9 @@ describe Cask::Cmd::List, :cask do described_class.run("local-transmission", "local-caffeine") }.to output(<<~EOS).to_stdout ==> Apps - #{Cask::Config.global.appdir.join("Transmission.app")} (#{Cask::Config.global.appdir.join("Transmission.app").abv}) + #{transmission.config.appdir.join("Transmission.app")} (#{transmission.config.appdir.join("Transmission.app").abv}) ==> Apps - Missing App: #{Cask::Config.global.appdir.join("Caffeine.app")} + Missing App: #{caffeine.config.appdir.join("Caffeine.app")} EOS end end diff --git a/Library/Homebrew/test/cask/cmd/uninstall_spec.rb b/Library/Homebrew/test/cask/cmd/uninstall_spec.rb index 215c9f9757..dc6a07e01d 100644 --- a/Library/Homebrew/test/cask/cmd/uninstall_spec.rb +++ b/Library/Homebrew/test/cask/cmd/uninstall_spec.rb @@ -51,9 +51,9 @@ describe Cask::Cmd::Uninstall, :cask do described_class.run("local-caffeine", "local-transmission") expect(caffeine).not_to be_installed - expect(Cask::Config.global.appdir.join("Transmission.app")).not_to exist + expect(caffeine.config.appdir.join("Transmission.app")).not_to exist expect(transmission).not_to be_installed - expect(Cask::Config.global.appdir.join("Caffeine.app")).not_to exist + expect(transmission.config.appdir.join("Caffeine.app")).not_to exist end it "calls `uninstall` before removing artifacts" do @@ -69,7 +69,7 @@ describe Cask::Cmd::Uninstall, :cask do }.not_to raise_error expect(cask).not_to be_installed - expect(Cask::Config.global.appdir.join("MyFancyApp.app")).not_to exist + expect(cask.config.appdir.join("MyFancyApp.app")).not_to exist end it "can uninstall Casks when the uninstall script is missing, but only when using `--force`" do @@ -79,7 +79,7 @@ describe Cask::Cmd::Uninstall, :cask do expect(cask).to be_installed - Cask::Config.global.appdir.join("MyFancyApp.app").rmtree + cask.config.appdir.join("MyFancyApp.app").rmtree expect { described_class.run("with-uninstall-script-app") } .to raise_error(Cask::CaskError, /uninstall script .* does not exist/) diff --git a/Library/Homebrew/test/cask/cmd/upgrade_spec.rb b/Library/Homebrew/test/cask/cmd/upgrade_spec.rb index b43bb6c9fa..16b2b82049 100644 --- a/Library/Homebrew/test/cask/cmd/upgrade_spec.rb +++ b/Library/Homebrew/test/cask/cmd/upgrade_spec.rb @@ -147,7 +147,7 @@ describe Cask::Cmd::Upgrade, :cask do it 'does not include the Casks with "auto_updates true" when the version did not change' do cask = Cask::CaskLoader.load("auto-updates") - cask_path = Cask::Config.global.appdir.join("MyFancyApp.app") + cask_path = cask.config.appdir.join("MyFancyApp.app") expect(cask).to be_installed expect(cask_path).to be_a_directory @@ -188,7 +188,7 @@ describe Cask::Cmd::Upgrade, :cask do it "restores the old Cask if the upgrade failed" do will_fail_if_upgraded = Cask::CaskLoader.load("will-fail-if-upgraded") - will_fail_if_upgraded_path = Cask::Config.global.appdir.join("container") + will_fail_if_upgraded_path = will_fail_if_upgraded.config.appdir.join("container") expect(will_fail_if_upgraded).to be_installed expect(will_fail_if_upgraded_path).to be_a_file @@ -206,7 +206,7 @@ describe Cask::Cmd::Upgrade, :cask do it "does not restore the old Cask if the upgrade failed pre-install" do bad_checksum = Cask::CaskLoader.load("bad-checksum") - bad_checksum_path = Cask::Config.global.appdir.join("Caffeine.app") + bad_checksum_path = bad_checksum.config.appdir.join("Caffeine.app") expect(bad_checksum).to be_installed expect(bad_checksum_path).to be_a_directory diff --git a/Library/Homebrew/test/cask/cmd/zap_spec.rb b/Library/Homebrew/test/cask/cmd/zap_spec.rb index f7071e0e1a..fd646477e8 100644 --- a/Library/Homebrew/test/cask/cmd/zap_spec.rb +++ b/Library/Homebrew/test/cask/cmd/zap_spec.rb @@ -23,8 +23,8 @@ describe Cask::Cmd::Zap, :cask do described_class.run("local-caffeine", "local-transmission") expect(caffeine).not_to be_installed - expect(Cask::Config.global.appdir.join("Caffeine.app")).not_to be_a_symlink + expect(caffeine.config.appdir.join("Caffeine.app")).not_to be_a_symlink expect(transmission).not_to be_installed - expect(Cask::Config.global.appdir.join("Transmission.app")).not_to be_a_symlink + expect(transmission.config.appdir.join("Transmission.app")).not_to be_a_symlink end end diff --git a/Library/Homebrew/test/cask/dsl_spec.rb b/Library/Homebrew/test/cask/dsl_spec.rb index 0532907f08..d26f237e27 100644 --- a/Library/Homebrew/test/cask/dsl_spec.rb +++ b/Library/Homebrew/test/cask/dsl_spec.rb @@ -468,7 +468,7 @@ describe Cask::DSL, :cask do let(:token) { "appdir-interpolation" } it "is allowed" do - expect(cask.artifacts.first.source).to eq(Cask::Config.global.appdir/"some/path") + expect(cask.artifacts.first.source).to eq(cask.config.appdir/"some/path") end end diff --git a/Library/Homebrew/test/cask/installer_spec.rb b/Library/Homebrew/test/cask/installer_spec.rb index ac19ed13bd..b9fbbf9171 100644 --- a/Library/Homebrew/test/cask/installer_spec.rb +++ b/Library/Homebrew/test/cask/installer_spec.rb @@ -10,7 +10,7 @@ describe Cask::Installer, :cask do Cask::Installer.new(caffeine).install expect(Cask::Caskroom.path.join("local-caffeine", caffeine.version)).to be_a_directory - expect(Cask::Config.global.appdir.join("Caffeine.app")).to be_a_directory + expect(caffeine.config.appdir.join("Caffeine.app")).to be_a_directory end it "works with dmg-based Casks" do @@ -19,7 +19,7 @@ describe Cask::Installer, :cask do Cask::Installer.new(asset).install expect(Cask::Caskroom.path.join("container-dmg", asset.version)).to be_a_directory - expect(Cask::Config.global.appdir.join("container")).to be_a_file + expect(asset.config.appdir.join("container")).to be_a_file end it "works with tar-gz-based Casks" do @@ -28,7 +28,7 @@ describe Cask::Installer, :cask do Cask::Installer.new(asset).install expect(Cask::Caskroom.path.join("container-tar-gz", asset.version)).to be_a_directory - expect(Cask::Config.global.appdir.join("container")).to be_a_file + expect(asset.config.appdir.join("container")).to be_a_file end it "works with xar-based Casks" do @@ -37,7 +37,7 @@ describe Cask::Installer, :cask do Cask::Installer.new(asset).install expect(Cask::Caskroom.path.join("container-xar", asset.version)).to be_a_directory - expect(Cask::Config.global.appdir.join("container")).to be_a_file + expect(asset.config.appdir.join("container")).to be_a_file end it "works with pure bzip2-based Casks" do @@ -46,7 +46,7 @@ describe Cask::Installer, :cask do Cask::Installer.new(asset).install expect(Cask::Caskroom.path.join("container-bzip2", asset.version)).to be_a_directory - expect(Cask::Config.global.appdir.join("container")).to be_a_file + expect(asset.config.appdir.join("container")).to be_a_file end it "works with pure gzip-based Casks" do @@ -55,7 +55,7 @@ describe Cask::Installer, :cask do Cask::Installer.new(asset).install expect(Cask::Caskroom.path.join("container-gzip", asset.version)).to be_a_directory - expect(Cask::Config.global.appdir.join("container")).to be_a_file + expect(asset.config.appdir.join("container")).to be_a_file end it "blows up on a bad checksum" do diff --git a/Library/Homebrew/test/cask/quarantine_spec.rb b/Library/Homebrew/test/cask/quarantine_spec.rb index b56818e228..c46706b00b 100644 --- a/Library/Homebrew/test/cask/quarantine_spec.rb +++ b/Library/Homebrew/test/cask/quarantine_spec.rb @@ -11,13 +11,11 @@ describe Cask::Quarantine, :cask do it "quarantines a nice fresh Cask" do Cask::Cmd::Install.run("local-transmission") - expect( - Cask::CaskLoader.load(cask_path("local-transmission")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("local-transmission")) - expect( - Cask::Config.global.appdir.join("Transmission.app"), - ).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("Transmission.app")).to be_quarantined end it "quarantines Cask fetches" do @@ -42,83 +40,81 @@ describe Cask::Quarantine, :cask do Cask::Cmd::Install.run("local-transmission") - expect( - Cask::CaskLoader.load(cask_path("local-transmission")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("local-transmission")) - expect(Cask::Config.global.appdir.join("Transmission.app")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("Transmission.app")).to be_quarantined end it "quarantines dmg-based Casks" do Cask::Cmd::Install.run("container-dmg") - expect( - Cask::CaskLoader.load(cask_path("container-dmg")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-dmg")) - expect(Cask::Config.global.appdir.join("container")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).to be_quarantined end it "quarantines tar-gz-based Casks" do Cask::Cmd::Install.run("container-tar-gz") - expect( - Cask::CaskLoader.load(cask_path("container-tar-gz")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-tar-gz")) - expect(Cask::Config.global.appdir.join("container")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).to be_quarantined end it "quarantines xar-based Casks" do Cask::Cmd::Install.run("container-xar") - expect( - Cask::CaskLoader.load(cask_path("container-xar")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-xar")) - expect(Cask::Config.global.appdir.join("container")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).to be_quarantined end it "quarantines pure bzip2-based Casks" do Cask::Cmd::Install.run("container-bzip2") - expect( - Cask::CaskLoader.load(cask_path("container-bzip2")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-bzip2")) - expect(Cask::Config.global.appdir.join("container")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).to be_quarantined end it "quarantines pure gzip-based Casks" do Cask::Cmd::Install.run("container-gzip") - expect( - Cask::CaskLoader.load(cask_path("container-gzip")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-gzip")) - expect(Cask::Config.global.appdir.join("container")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).to be_quarantined end it "quarantines the pkg in naked-pkg-based Casks" do Cask::Cmd::Install.run("container-pkg") - naked_pkg = Cask::CaskLoader.load(cask_path("container-pkg")) + cask = Cask::CaskLoader.load(cask_path("container-pkg")) - expect(naked_pkg).to be_installed + expect(cask).to be_installed - expect( - Cask::Caskroom.path.join("container-pkg", naked_pkg.version, "container.pkg"), - ).to be_quarantined + expect(cask.staged_path/"container.pkg").to be_quarantined end it "quarantines a nested container" do Cask::Cmd::Install.run("nested-app") - expect( - Cask::CaskLoader.load(cask_path("nested-app")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("nested-app")) - expect(Cask::Config.global.appdir.join("MyNestedApp.app")).to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("MyNestedApp.app")).to be_quarantined end end @@ -126,11 +122,11 @@ describe Cask::Quarantine, :cask do it "does not quarantine even a nice, fresh Cask" do Cask::Cmd::Install.run("local-transmission", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("local-transmission")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("local-transmission")) - expect(Cask::Config.global.appdir.join("Transmission.app")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("Transmission.app")).not_to be_quarantined end it "does not quarantine Cask fetches" do @@ -155,61 +151,61 @@ describe Cask::Quarantine, :cask do Cask::Cmd::Install.run("local-transmission", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("local-transmission")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("local-transmission")) - expect(Cask::Config.global.appdir.join("Transmission.app")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("Transmission.app")).not_to be_quarantined end it "does not quarantine dmg-based Casks" do Cask::Cmd::Install.run("container-dmg", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("container-dmg")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-dmg")) - expect(Cask::Config.global.appdir.join("container")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).not_to be_quarantined end it "does not quarantine tar-gz-based Casks" do Cask::Cmd::Install.run("container-tar-gz", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("container-tar-gz")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-tar-gz")) - expect(Cask::Config.global.appdir.join("container")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).not_to be_quarantined end it "does not quarantine xar-based Casks" do Cask::Cmd::Install.run("container-xar", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("container-xar")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-xar")) - expect(Cask::Config.global.appdir.join("container")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).not_to be_quarantined end it "does not quarantine pure bzip2-based Casks" do Cask::Cmd::Install.run("container-bzip2", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("container-bzip2")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-bzip2")) - expect(Cask::Config.global.appdir.join("container")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).not_to be_quarantined end it "does not quarantine pure gzip-based Casks" do Cask::Cmd::Install.run("container-gzip", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("container-gzip")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("container-gzip")) - expect(Cask::Config.global.appdir.join("container")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("container")).not_to be_quarantined end it "does not quarantine the pkg in naked-pkg-based Casks" do @@ -227,11 +223,11 @@ describe Cask::Quarantine, :cask do it "does not quarantine a nested container" do Cask::Cmd::Install.run("nested-app", "--no-quarantine") - expect( - Cask::CaskLoader.load(cask_path("nested-app")), - ).to be_installed + cask = Cask::CaskLoader.load(cask_path("nested-app")) - expect(Cask::Config.global.appdir.join("MyNestedApp.app")).not_to be_quarantined + expect(cask).to be_installed + + expect(cask.config.appdir.join("MyNestedApp.app")).not_to be_quarantined end end end diff --git a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb index f4432f5f06..bb228b66b4 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb @@ -14,7 +14,7 @@ HOMEBREW_CASK_DIRS = { RSpec.shared_context "Homebrew Cask", :needs_macos do before do HOMEBREW_CASK_DIRS.each do |method, path| - allow(Cask::Config.global).to receive(method).and_return(path) + Cask::Config.global.send("#{method}=", path) end end From fda6e0cab3901978f632358295db4a62413fd281 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 3 Feb 2019 02:40:27 +0100 Subject: [PATCH 2/9] Split cask config into three tiers. --- Library/Homebrew/cask/cmd.rb | 10 +-- Library/Homebrew/cask/config.rb | 79 ++++++++++++++----- .../Homebrew/test/cask/cmd/options_spec.rb | 30 ------- Library/Homebrew/test/cask/cmd_spec.rb | 4 +- .../spec/shared_context/homebrew_cask.rb | 40 ++++++---- 5 files changed, 90 insertions(+), 73 deletions(-) diff --git a/Library/Homebrew/cask/cmd.rb b/Library/Homebrew/cask/cmd.rb index a064a30333..f40b55d599 100644 --- a/Library/Homebrew/cask/cmd.rb +++ b/Library/Homebrew/cask/cmd.rb @@ -186,16 +186,14 @@ module Cask end def process_options(*args) - all_args = Shellwords.shellsplit(ENV["HOMEBREW_CASK_OPTS"] || "") + args - non_options = [] - if idx = all_args.index("--") - non_options += all_args.drop(idx) - all_args = all_args.first(idx) + if idx = args.index("--") + non_options += args.drop(idx) + args = args.first(idx) end - remaining = all_args.select do |arg| + remaining = args.select do |arg| begin !process_arguments([arg]).empty? rescue OptionParser::InvalidOption, OptionParser::MissingArgument, OptionParser::AmbiguousOption diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index ed40737b47..5eea14c647 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -1,27 +1,34 @@ require "json" +require "extend/hash_validator" +using HashValidator + module Cask - class Config < DelegateClass(Hash) + class Config DEFAULT_DIRS = { - appdir: "/Applications", - prefpanedir: "~/Library/PreferencePanes", - qlplugindir: "~/Library/QuickLook", - dictionarydir: "~/Library/Dictionaries", - fontdir: "~/Library/Fonts", - colorpickerdir: "~/Library/ColorPickers", - servicedir: "~/Library/Services", - input_methoddir: "~/Library/Input Methods", - internet_plugindir: "~/Library/Internet Plug-Ins", - audio_unit_plugindir: "~/Library/Audio/Plug-Ins/Components", - vst_plugindir: "~/Library/Audio/Plug-Ins/VST", - vst3_plugindir: "~/Library/Audio/Plug-Ins/VST3", - screen_saverdir: "~/Library/Screen Savers", + appdir: Pathname("/Applications").expand_path, + prefpanedir: Pathname("~/Library/PreferencePanes").expand_path, + qlplugindir: Pathname("~/Library/QuickLook").expand_path, + dictionarydir: Pathname("~/Library/Dictionaries").expand_path, + fontdir: Pathname("~/Library/Fonts").expand_path, + colorpickerdir: Pathname("~/Library/ColorPickers").expand_path, + servicedir: Pathname("~/Library/Services").expand_path, + input_methoddir: Pathname("~/Library/Input Methods").expand_path, + internet_plugindir: Pathname("~/Library/Internet Plug-Ins").expand_path, + audio_unit_plugindir: Pathname("~/Library/Audio/Plug-Ins/Components").expand_path, + vst_plugindir: Pathname("~/Library/Audio/Plug-Ins/VST").expand_path, + vst3_plugindir: Pathname("~/Library/Audio/Plug-Ins/VST3").expand_path, + screen_saverdir: Pathname("~/Library/Screen Savers").expand_path, }.freeze def self.global @global ||= new end + def self.clear + @global = nil + end + def self.for_cask(cask) if cask.config_path.exist? from_file(cask.config_path) @@ -37,11 +44,33 @@ module Cask raise e, "Cannot parse #{path}: #{e}", e.backtrace end - new(Hash[config.map { |k, v| [k.to_sym, v] }]) + new( + default: config.fetch("default", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h, + env: config.fetch("env", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h, + explicit: config.fetch("explicit", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h, + ) end - def initialize(**dirs) - super(Hash[DEFAULT_DIRS.map { |(k, v)| [k, Pathname(dirs.fetch(k, v)).expand_path] }]) + attr_accessor :explicit + + def initialize(default: nil, env: nil, explicit: {}) + env&.assert_valid_keys!(*DEFAULT_DIRS.keys) + explicit.assert_valid_keys!(*DEFAULT_DIRS.keys) + + @default = default + @env = env + @explicit = explicit.map { |(k, v)| [k.to_sym, Pathname(v).expand_path] }.to_h + end + + def default + @default ||= DEFAULT_DIRS + end + + def env + @env ||= Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) + .map { |arg| arg.split("=", 2) } + .map { |(flag, value)| [flag.sub(/^\-\-/, "").to_sym, Pathname(value).expand_path] } + .to_h end def binarydir @@ -50,14 +79,26 @@ module Cask DEFAULT_DIRS.keys.each do |dir| define_method(dir) do - self[dir] + explicit.fetch(dir, env.fetch(dir, default.fetch(dir))) end define_method(:"#{dir}=") do |path| - self[dir] = Pathname(path).expand_path + explicit[dir] = Pathname(path).expand_path end end + def merge(other) + self.class.new(**other.explicit.merge(explicit)) + end + + def to_json(*args) + { + default: default, + env: env, + explicit: explicit, + }.to_json(*args) + end + def write(path) path.atomic_write(to_json) end diff --git a/Library/Homebrew/test/cask/cmd/options_spec.rb b/Library/Homebrew/test/cask/cmd/options_spec.rb index ec565b57fc..0f254573f6 100644 --- a/Library/Homebrew/test/cask/cmd/options_spec.rb +++ b/Library/Homebrew/test/cask/cmd/options_spec.rb @@ -1,15 +1,11 @@ describe Cask::Cmd, :cask do it "supports setting the appdir" do - allow(Cask::Config.global).to receive(:appdir).and_call_original - described_class.new.process_options("help", "--appdir=/some/path/foo") expect(Cask::Config.global.appdir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the appdir from ENV" do - allow(Cask::Config.global).to receive(:appdir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/some/path/bar" described_class.new.process_options("help") @@ -18,16 +14,12 @@ describe Cask::Cmd, :cask do end it "supports setting the prefpanedir" do - allow(Cask::Config.global).to receive(:prefpanedir).and_call_original - described_class.new.process_options("help", "--prefpanedir=/some/path/foo") expect(Cask::Config.global.prefpanedir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the prefpanedir from ENV" do - allow(Cask::Config.global).to receive(:prefpanedir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--prefpanedir=/some/path/bar" described_class.new.process_options("help") @@ -36,16 +28,12 @@ describe Cask::Cmd, :cask do end it "supports setting the qlplugindir" do - allow(Cask::Config.global).to receive(:qlplugindir).and_call_original - described_class.new.process_options("help", "--qlplugindir=/some/path/foo") expect(Cask::Config.global.qlplugindir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the qlplugindir from ENV" do - allow(Cask::Config.global).to receive(:qlplugindir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--qlplugindir=/some/path/bar" described_class.new.process_options("help") @@ -54,16 +42,12 @@ describe Cask::Cmd, :cask do end it "supports setting the colorpickerdir" do - allow(Cask::Config.global).to receive(:colorpickerdir).and_call_original - described_class.new.process_options("help", "--colorpickerdir=/some/path/foo") expect(Cask::Config.global.colorpickerdir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the colorpickerdir from ENV" do - allow(Cask::Config.global).to receive(:colorpickerdir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--colorpickerdir=/some/path/bar" described_class.new.process_options("help") @@ -72,16 +56,12 @@ describe Cask::Cmd, :cask do end it "supports setting the dictionarydir" do - allow(Cask::Config.global).to receive(:dictionarydir).and_call_original - described_class.new.process_options("help", "--dictionarydir=/some/path/foo") expect(Cask::Config.global.dictionarydir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the dictionarydir from ENV" do - allow(Cask::Config.global).to receive(:dictionarydir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--dictionarydir=/some/path/bar" described_class.new.process_options("help") @@ -90,16 +70,12 @@ describe Cask::Cmd, :cask do end it "supports setting the fontdir" do - allow(Cask::Config.global).to receive(:fontdir).and_call_original - described_class.new.process_options("help", "--fontdir=/some/path/foo") expect(Cask::Config.global.fontdir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the fontdir from ENV" do - allow(Cask::Config.global).to receive(:fontdir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--fontdir=/some/path/bar" described_class.new.process_options("help") @@ -108,16 +84,12 @@ describe Cask::Cmd, :cask do end it "supports setting the servicedir" do - allow(Cask::Config.global).to receive(:servicedir).and_call_original - described_class.new.process_options("help", "--servicedir=/some/path/foo") expect(Cask::Config.global.servicedir).to eq(Pathname.new("/some/path/foo")) end it "supports setting the servicedir from ENV" do - allow(Cask::Config.global).to receive(:servicedir).and_call_original - ENV["HOMEBREW_CASK_OPTS"] = "--servicedir=/some/path/bar" described_class.new.process_options("help") @@ -126,8 +98,6 @@ describe Cask::Cmd, :cask do end it "allows additional options to be passed through" do - allow(Cask::Config.global).to receive(:appdir).and_call_original - rest = described_class.new.process_options("edit", "foo", "--create", "--appdir=/some/path/qux") expect(Cask::Config.global.appdir).to eq(Pathname.new("/some/path/qux")) diff --git a/Library/Homebrew/test/cask/cmd_spec.rb b/Library/Homebrew/test/cask/cmd_spec.rb index 987fa2ecb9..ee2a68fd46 100644 --- a/Library/Homebrew/test/cask/cmd_spec.rb +++ b/Library/Homebrew/test/cask/cmd_spec.rb @@ -56,9 +56,7 @@ describe Cask::Cmd, :cask do end it "respects the env variable when choosing what appdir to create" do - allow(ENV).to receive(:[]).and_call_original - allow(ENV).to receive(:[]).with("HOMEBREW_CASK_OPTS").and_return("--appdir=/custom/appdir") - allow(Cask::Config.global).to receive(:appdir).and_call_original + ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/custom/appdir" described_class.run("noop") diff --git a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb index bb228b66b4..06285c5969 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb @@ -4,25 +4,34 @@ require "test/support/helper/cask/fake_system_command" require "test/support/helper/cask/install_helper" require "test/support/helper/cask/never_sudo_system_command" -HOMEBREW_CASK_DIRS = { - appdir: Pathname.new(TEST_TMPDIR).join("cask-appdir"), - prefpanedir: Pathname.new(TEST_TMPDIR).join("cask-prefpanedir"), - qlplugindir: Pathname.new(TEST_TMPDIR).join("cask-qlplugindir"), - servicedir: Pathname.new(TEST_TMPDIR).join("cask-servicedir"), -}.freeze +module Cask + class Config + remove_const :DEFAULT_DIRS + + DEFAULT_DIRS = { + appdir: Pathname.new(TEST_TMPDIR).join("cask-appdir"), + prefpanedir: Pathname.new(TEST_TMPDIR).join("cask-prefpanedir"), + qlplugindir: Pathname.new(TEST_TMPDIR).join("cask-qlplugindir"), + dictionarydir: Pathname.new(TEST_TMPDIR).join("cask-dictionarydir"), + fontdir: Pathname.new(TEST_TMPDIR).join("cask-fontdir"), + colorpickerdir: Pathname.new(TEST_TMPDIR).join("cask-colorpickerdir"), + servicedir: Pathname.new(TEST_TMPDIR).join("cask-servicedir"), + input_methoddir: Pathname.new(TEST_TMPDIR).join("cask-input_methoddir"), + internet_plugindir: Pathname.new(TEST_TMPDIR).join("cask-internet_plugindir"), + audio_unit_plugindir: Pathname.new(TEST_TMPDIR).join("cask-audio_unit_plugindir"), + vst_plugindir: Pathname.new(TEST_TMPDIR).join("cask-vst_plugindir"), + vst3_plugindir: Pathname.new(TEST_TMPDIR).join("cask-vst3_plugindir"), + screen_saverdir: Pathname.new(TEST_TMPDIR).join("cask-screen_saverdir"), + }.freeze + end +end RSpec.shared_context "Homebrew Cask", :needs_macos do - before do - HOMEBREW_CASK_DIRS.each do |method, path| - Cask::Config.global.send("#{method}=", path) - end - end - around do |example| third_party_tap = Tap.fetch("third-party", "tap") - begin - HOMEBREW_CASK_DIRS.values.each(&:mkpath) + begin + Cask::Config::DEFAULT_DIRS.values.each(&:mkpath) Cask::Config.global.binarydir.mkpath Tap.default_cask_tap.tap do |tap| @@ -37,11 +46,12 @@ RSpec.shared_context "Homebrew Cask", :needs_macos do example.run ensure - FileUtils.rm_rf HOMEBREW_CASK_DIRS.values + FileUtils.rm_rf Cask::Config::DEFAULT_DIRS.values FileUtils.rm_rf [Cask::Config.global.binarydir, Cask::Caskroom.path, Cask::Cache.path] Tap.default_cask_tap.path.unlink third_party_tap.path.unlink FileUtils.rm_rf third_party_tap.path.parent + Cask::Config.clear end end end From 190ff7558af37c4d2e76323482d38b3c1fc4f47c Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 3 Feb 2019 13:03:16 +0100 Subject: [PATCH 3/9] Use saved cask config for reinstall/upgrade/uninstall. --- .../cask/artifact/abstract_artifact.rb | 7 +++-- Library/Homebrew/cask/cask.rb | 26 ++++++++++++------- Library/Homebrew/cask/cmd/upgrade.rb | 8 +++++- Library/Homebrew/cask/config.rb | 2 +- Library/Homebrew/cask/dsl.rb | 3 +-- Library/Homebrew/cask/installer.rb | 6 +++++ .../test/cask/artifact/installer_spec.rb | 2 +- Library/Homebrew/test/cask/cmd/audit_spec.rb | 2 +- 8 files changed, 38 insertions(+), 18 deletions(-) diff --git a/Library/Homebrew/cask/artifact/abstract_artifact.rb b/Library/Homebrew/cask/artifact/abstract_artifact.rb index f8ceb22001..66dea2b617 100644 --- a/Library/Homebrew/cask/artifact/abstract_artifact.rb +++ b/Library/Homebrew/cask/artifact/abstract_artifact.rb @@ -94,11 +94,14 @@ module Cask [executable, arguments] end - attr_reader :cask, :config + attr_reader :cask def initialize(cask) @cask = cask - @config = cask.config + end + + def config + cask.config end def to_s diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index 4336cff60a..cfeef216b7 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -11,7 +11,7 @@ module Cask extend Searchable include Metadata - attr_reader :token, :sourcefile_path + attr_reader :token, :sourcefile_path, :config def self.each return to_enum unless block_given? @@ -31,15 +31,21 @@ module Cask @tap end - def initialize(token, sourcefile_path: nil, tap: nil, config: nil, &block) + def initialize(token, sourcefile_path: nil, tap: nil, &block) @token = token @sourcefile_path = sourcefile_path @tap = tap - @config = config - @dsl = DSL.new(self) - return unless block_given? + @block = block + self.config = Config.for_cask(self) + end - @dsl.instance_eval(&block) + def config=(config) + @config = config + + @dsl = DSL.new(self) + return unless @block + + @dsl.instance_eval(&@block) @dsl.language_eval end @@ -77,14 +83,14 @@ module Cask metadata_master_container_path.join(*installed_version, "Casks", "#{token}.rb") end - def config - @config ||= Config.for_cask(self) - end - def config_path metadata_master_container_path/"dirs.json" end + def caskroom_path + @caskroom_path ||= Caskroom.path.join(token) + end + def outdated?(greedy = false) !outdated_versions(greedy).empty? end diff --git a/Library/Homebrew/cask/cmd/upgrade.rb b/Library/Homebrew/cask/cmd/upgrade.rb index d12a03630e..06e880bd12 100644 --- a/Library/Homebrew/cask/cmd/upgrade.rb +++ b/Library/Homebrew/cask/cmd/upgrade.rb @@ -1,3 +1,5 @@ +require "cask/config" + module Cask class Cmd class Upgrade < AbstractCommand @@ -44,13 +46,17 @@ module Cask old_cask = CaskLoader.load(old_cask.installed_caskfile) + old_config = old_cask.config + old_cask_installer = Installer.new(old_cask, binaries: binaries?, verbose: verbose?, force: force?, upgrade: true) - new_cask = CaskLoader.load(old_cask.to_s) + new_cask = CaskLoader.load(old_cask.token) + + new_cask.config = Config.global.merge(old_config) new_cask_installer = Installer.new(new_cask, binaries: binaries?, diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index 5eea14c647..082afd3a71 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -88,7 +88,7 @@ module Cask end def merge(other) - self.class.new(**other.explicit.merge(explicit)) + self.class.new(explicit: other.explicit.merge(explicit)) end def to_json(*args) diff --git a/Library/Homebrew/cask/dsl.rb b/Library/Homebrew/cask/dsl.rb index 45ce56be56..5a9b540a6b 100644 --- a/Library/Homebrew/cask/dsl.rb +++ b/Library/Homebrew/cask/dsl.rb @@ -57,7 +57,6 @@ module Cask :appcast, :artifacts, :auto_updates, - :caskroom_path, :caveats, :conflicts_with, :container, @@ -226,7 +225,7 @@ module Cask end def caskroom_path - @caskroom_path ||= Caskroom.path.join(token) + @cask.caskroom_path end def staged_path diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 74f1664972..78c7aa3f4a 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -4,6 +4,7 @@ require "formula_installer" require "unpack_strategy" require "cask/cask_dependencies" +require "cask/config" require "cask/download" require "cask/staged" require "cask/verify" @@ -79,6 +80,8 @@ module Cask def install odebug "Cask::Installer#install" + old_config = @cask.config + if @cask.installed? && !force? && !reinstall? && !upgrade? raise CaskAlreadyInstalledError, @cask end @@ -92,6 +95,9 @@ module Cask oh1 "Installing Cask #{Formatter.identifier(@cask)}" opoo "macOS's Gatekeeper has been disabled for this Cask" unless quarantine? stage + + @cask.config = Config.global.merge(old_config) + install_artifacts unless @cask.tap&.private? diff --git a/Library/Homebrew/test/cask/artifact/installer_spec.rb b/Library/Homebrew/test/cask/artifact/installer_spec.rb index d2aab5ba5d..8952fd6152 100644 --- a/Library/Homebrew/test/cask/artifact/installer_spec.rb +++ b/Library/Homebrew/test/cask/artifact/installer_spec.rb @@ -2,7 +2,7 @@ describe Cask::Artifact::Installer, :cask do subject(:installer) { described_class.new(cask, **args) } let(:staged_path) { mktmpdir } - let(:cask) { instance_double(Cask::Cask, staged_path: staged_path, config: nil) } + let(:cask) { instance_double(Cask::Cask, staged_path: staged_path) } let(:command) { SystemCommand } diff --git a/Library/Homebrew/test/cask/cmd/audit_spec.rb b/Library/Homebrew/test/cask/cmd/audit_spec.rb index b3b818a834..529019b0ca 100644 --- a/Library/Homebrew/test/cask/cmd/audit_spec.rb +++ b/Library/Homebrew/test/cask/cmd/audit_spec.rb @@ -1,7 +1,7 @@ require_relative "shared_examples/invalid_option" describe Cask::Cmd::Audit, :cask do - let(:cask) { Cask::Cask.new(nil) } + let(:cask) { Cask::Cask.new("cask") } it_behaves_like "a command that handles invalid options" From 707cb33346c0116c3607483a381422855582415a Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 3 Feb 2019 15:57:51 +0100 Subject: [PATCH 4/9] Add tests for `Cask::Config`. --- Library/Homebrew/test/cask/config_spec.rb | 51 +++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 Library/Homebrew/test/cask/config_spec.rb diff --git a/Library/Homebrew/test/cask/config_spec.rb b/Library/Homebrew/test/cask/config_spec.rb new file mode 100644 index 0000000000..f160104bbe --- /dev/null +++ b/Library/Homebrew/test/cask/config_spec.rb @@ -0,0 +1,51 @@ +describe Cask::Config, :cask do + subject(:config) { described_class.new } + + describe "#default" do + it "returns the default directories" do + expect(config.default[:appdir]).to eq(Pathname(TEST_TMPDIR).join("cask-appdir")) + end + end + + describe "#appdir" do + it "returns the default value if no HOMEBREW_CASK_OPTS is unset" do + expect(config.appdir).to eq(Pathname(TEST_TMPDIR).join("cask-appdir")) + end + + specify "environment overwrites default" do + ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/path/to/apps" + + expect(config.appdir).to eq(Pathname("/path/to/apps")) + end + + specify "specific overwrites default" do + config = described_class.new(explicit: { appdir: "/explicit/path/to/apps" }) + + expect(config.appdir).to eq(Pathname("/explicit/path/to/apps")) + end + + specify "explicit overwrites environment" do + ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/path/to/apps" + + config = described_class.new(explicit: { appdir: "/explicit/path/to/apps" }) + + expect(config.appdir).to eq(Pathname("/explicit/path/to/apps")) + end + end + + describe "#env" do + it "returns directories specified with the HOMEBREW_CASK_OPTS variable" do + ENV["HOMEBREW_CASK_OPTS"] = "--appdir=/path/to/apps" + + expect(config.env).to eq(appdir: Pathname("/path/to/apps")) + end + end + + describe "#explicit" do + let(:config) { described_class.new(explicit: { appdir: "/explicit/path/to/apps" }) } + + it "returns directories explicitly given as arguments" do + expect(config.explicit[:appdir]).to eq(Pathname("/explicit/path/to/apps")) + end + end +end From defbf7d74c0c47ccecfef073dc80d89d5b185306 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sun, 3 Feb 2019 17:41:51 +0100 Subject: [PATCH 5/9] =?UTF-8?q?Don=E2=80=99t=20ignore=20other=20arguments?= =?UTF-8?q?=20in=20`HOMEBREW=5FCASK=5FOPTS`.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Library/Homebrew/cask/cmd.rb | 13 +++++++++---- Library/Homebrew/cask/config.rb | 1 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/cask/cmd.rb b/Library/Homebrew/cask/cmd.rb index f40b55d599..5a1c65b2e0 100644 --- a/Library/Homebrew/cask/cmd.rb +++ b/Library/Homebrew/cask/cmd.rb @@ -186,14 +186,19 @@ module Cask end def process_options(*args) + exclude_regex = /^\-\-#{Regexp.union(*Config::DEFAULT_DIRS.keys.map(&Regexp.public_method(:escape)))}=/ + + all_args = Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) + .reject { |arg| arg.match?(exclude_regex) } + args + non_options = [] - if idx = args.index("--") - non_options += args.drop(idx) - args = args.first(idx) + if idx = all_args.index("--") + non_options += all_args.drop(idx) + all_args = all_args.first(idx) end - remaining = args.select do |arg| + remaining = all_args.select do |arg| begin !process_arguments([arg]).empty? rescue OptionParser::InvalidOption, OptionParser::MissingArgument, OptionParser::AmbiguousOption diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index 082afd3a71..a575bf27da 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -68,6 +68,7 @@ module Cask def env @env ||= Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) + .select { |arg| arg.include?("=") } .map { |arg| arg.split("=", 2) } .map { |(flag, value)| [flag.sub(/^\-\-/, "").to_sym, Pathname(value).expand_path] } .to_h From bcdb4a3f3273eb63076c6714500469767efdf688 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Tue, 5 Feb 2019 16:08:29 +0100 Subject: [PATCH 6/9] Add `Cask::Config::canonicalize`. --- Library/Homebrew/cask/config.rb | 59 ++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index a575bf27da..eba5ecfe0b 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -6,19 +6,19 @@ using HashValidator module Cask class Config DEFAULT_DIRS = { - appdir: Pathname("/Applications").expand_path, - prefpanedir: Pathname("~/Library/PreferencePanes").expand_path, - qlplugindir: Pathname("~/Library/QuickLook").expand_path, - dictionarydir: Pathname("~/Library/Dictionaries").expand_path, - fontdir: Pathname("~/Library/Fonts").expand_path, - colorpickerdir: Pathname("~/Library/ColorPickers").expand_path, - servicedir: Pathname("~/Library/Services").expand_path, - input_methoddir: Pathname("~/Library/Input Methods").expand_path, - internet_plugindir: Pathname("~/Library/Internet Plug-Ins").expand_path, - audio_unit_plugindir: Pathname("~/Library/Audio/Plug-Ins/Components").expand_path, - vst_plugindir: Pathname("~/Library/Audio/Plug-Ins/VST").expand_path, - vst3_plugindir: Pathname("~/Library/Audio/Plug-Ins/VST3").expand_path, - screen_saverdir: Pathname("~/Library/Screen Savers").expand_path, + appdir: "/Applications", + prefpanedir: "~/Library/PreferencePanes", + qlplugindir: "~/Library/QuickLook", + dictionarydir: "~/Library/Dictionaries", + fontdir: "~/Library/Fonts", + colorpickerdir: "~/Library/ColorPickers", + servicedir: "~/Library/Services", + input_methoddir: "~/Library/Input Methods", + internet_plugindir: "~/Library/Internet Plug-Ins", + audio_unit_plugindir: "~/Library/Audio/Plug-Ins/Components", + vst_plugindir: "~/Library/Audio/Plug-Ins/VST", + vst3_plugindir: "~/Library/Audio/Plug-Ins/VST3", + screen_saverdir: "~/Library/Screen Savers", }.freeze def self.global @@ -45,33 +45,38 @@ module Cask end new( - default: config.fetch("default", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h, - env: config.fetch("env", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h, - explicit: config.fetch("explicit", {}).map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h, + default: config.fetch("default", {}), + env: config.fetch("env", {}), + explicit: config.fetch("explicit", {}), ) end + def self.canonicalize(config) + config.map { |k, v| [k.to_sym, Pathname(v).expand_path] }.to_h + end + attr_accessor :explicit def initialize(default: nil, env: nil, explicit: {}) - env&.assert_valid_keys!(*DEFAULT_DIRS.keys) - explicit.assert_valid_keys!(*DEFAULT_DIRS.keys) + @default = self.class.canonicalize(default) if default + @env = self.class.canonicalize(env) if env + @explicit = self.class.canonicalize(explicit) - @default = default - @env = env - @explicit = explicit.map { |(k, v)| [k.to_sym, Pathname(v).expand_path] }.to_h + @env&.assert_valid_keys!(*DEFAULT_DIRS.keys) + @explicit.assert_valid_keys!(*DEFAULT_DIRS.keys) end def default - @default ||= DEFAULT_DIRS + @default ||= self.class.canonicalize(DEFAULT_DIRS) end def env - @env ||= Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) - .select { |arg| arg.include?("=") } - .map { |arg| arg.split("=", 2) } - .map { |(flag, value)| [flag.sub(/^\-\-/, "").to_sym, Pathname(value).expand_path] } - .to_h + @env ||= self.class.canonicalize( + Shellwords.shellsplit(ENV.fetch("HOMEBREW_CASK_OPTS", "")) + .select { |arg| arg.include?("=") } + .map { |arg| arg.split("=", 2) } + .map { |(flag, value)| [flag.sub(/^\-\-/, ""), value] }, + ) end def binarydir From 4c6d0ba06994a5c67b283a02270acfd2b80f2722 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 8 Feb 2019 03:00:27 +0100 Subject: [PATCH 7/9] Rename `dirs.json` to `config.json`. --- Library/Homebrew/cask/cask.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Library/Homebrew/cask/cask.rb b/Library/Homebrew/cask/cask.rb index cfeef216b7..68f2cd3709 100644 --- a/Library/Homebrew/cask/cask.rb +++ b/Library/Homebrew/cask/cask.rb @@ -84,7 +84,7 @@ module Cask end def config_path - metadata_master_container_path/"dirs.json" + metadata_master_container_path/"config.json" end def caskroom_path From 3bf7e5bce572e22734a727f27c952df2116933bb Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 9 Feb 2019 02:19:01 +0100 Subject: [PATCH 8/9] Remove `Cask::Config#write`. --- Library/Homebrew/cask/config.rb | 4 ---- Library/Homebrew/cask/installer.rb | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/Library/Homebrew/cask/config.rb b/Library/Homebrew/cask/config.rb index eba5ecfe0b..404370e689 100644 --- a/Library/Homebrew/cask/config.rb +++ b/Library/Homebrew/cask/config.rb @@ -104,9 +104,5 @@ module Cask explicit: explicit, }.to_json(*args) end - - def write(path) - path.atomic_write(to_json) - end end end diff --git a/Library/Homebrew/cask/installer.rb b/Library/Homebrew/cask/installer.rb index 78c7aa3f4a..88b8e83193 100644 --- a/Library/Homebrew/cask/installer.rb +++ b/Library/Homebrew/cask/installer.rb @@ -391,7 +391,7 @@ module Cask end def save_config_file - @cask.config.write(@cask.config_path) + @cask.config_path.atomic_write(@cask.config.to_json) end def uninstall From 78d4373543235d6822314c895785de0b5e582e90 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 9 Feb 2019 02:39:04 +0100 Subject: [PATCH 9/9] Simplify `DEFAULT_DIRS`. --- .../spec/shared_context/homebrew_cask.rb | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb index 06285c5969..60596a8997 100644 --- a/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb +++ b/Library/Homebrew/test/support/helper/spec/shared_context/homebrew_cask.rb @@ -9,19 +9,19 @@ module Cask remove_const :DEFAULT_DIRS DEFAULT_DIRS = { - appdir: Pathname.new(TEST_TMPDIR).join("cask-appdir"), - prefpanedir: Pathname.new(TEST_TMPDIR).join("cask-prefpanedir"), - qlplugindir: Pathname.new(TEST_TMPDIR).join("cask-qlplugindir"), - dictionarydir: Pathname.new(TEST_TMPDIR).join("cask-dictionarydir"), - fontdir: Pathname.new(TEST_TMPDIR).join("cask-fontdir"), - colorpickerdir: Pathname.new(TEST_TMPDIR).join("cask-colorpickerdir"), - servicedir: Pathname.new(TEST_TMPDIR).join("cask-servicedir"), - input_methoddir: Pathname.new(TEST_TMPDIR).join("cask-input_methoddir"), - internet_plugindir: Pathname.new(TEST_TMPDIR).join("cask-internet_plugindir"), - audio_unit_plugindir: Pathname.new(TEST_TMPDIR).join("cask-audio_unit_plugindir"), - vst_plugindir: Pathname.new(TEST_TMPDIR).join("cask-vst_plugindir"), - vst3_plugindir: Pathname.new(TEST_TMPDIR).join("cask-vst3_plugindir"), - screen_saverdir: Pathname.new(TEST_TMPDIR).join("cask-screen_saverdir"), + appdir: Pathname(TEST_TMPDIR)/"cask-appdir", + prefpanedir: Pathname(TEST_TMPDIR)/"cask-prefpanedir", + qlplugindir: Pathname(TEST_TMPDIR)/"cask-qlplugindir", + dictionarydir: Pathname(TEST_TMPDIR)/"cask-dictionarydir", + fontdir: Pathname(TEST_TMPDIR)/"cask-fontdir", + colorpickerdir: Pathname(TEST_TMPDIR)/"cask-colorpickerdir", + servicedir: Pathname(TEST_TMPDIR)/"cask-servicedir", + input_methoddir: Pathname(TEST_TMPDIR)/"cask-input_methoddir", + internet_plugindir: Pathname(TEST_TMPDIR)/"cask-internet_plugindir", + audio_unit_plugindir: Pathname(TEST_TMPDIR)/"cask-audio_unit_plugindir", + vst_plugindir: Pathname(TEST_TMPDIR)/"cask-vst_plugindir", + vst3_plugindir: Pathname(TEST_TMPDIR)/"cask-vst3_plugindir", + screen_saverdir: Pathname(TEST_TMPDIR)/"cask-screen_saverdir", }.freeze end end